Messages in this thread | | | Subject | Re: [PATCH v2 18/29] nvmem: Add Realtek DHC eFuse driver | From | Andreas Färber <> | Date | Tue, 23 Jun 2020 13:15:23 +0200 |
| |
Hi Srini,
Am 23.06.20 um 11:32 schrieb Srinivas Kandagatla: > On 23/06/2020 03:50, Andreas Färber wrote: >> Implement enough of a read-only nvmem driver to easily read efuse cells. >> >> Cc: Cheng-Yu Lee <cylee12@realtek.com> >> Signed-off-by: Andreas Färber <afaerber@suse.de> >> --- > > This patch itself looks okay to me, I will apply this once DT patches > are Reviewed/applied by DT maintainers!
Thanks - let's give the Realtek engineers some time to review, too:
* Driver naming - new [rtk-]dhc (Stanley) vs. in-tree rtd1195 elsewhere.
* My other driver was previously reading u32-sized registers directly, whereas here I switched to byte-sized reads based on other in-tree nvmem drivers. Downstream driver seems inconsistent wrt .word_size: https://github.com/BPI-SINOVOIP/BPI-M4-bsp/blob/master/linux-rtk/drivers/nvmem/rtk-efuse.c#L191
* RTD1619 (RTD1319, too?) may need special handling and therefore its own DT compatible: There's a magic OTP_CTRL register write downstream that I am lacking documentation w/ names&explanations and use case for. https://github.com/BPI-SINOVOIP/BPI-M4-bsp/blob/master/linux-rtk/drivers/nvmem/rtk-efuse.c#L216 That might obviously affect the binding, too, requiring oneOf - could be changed in a later step though.
I would take the .dts patches through my linux-realtek.git once the binding is approved.
* The downstream DTs have nvmem-cells and nvmem-cell-names properties in the efuse node directly, which I regarded as unnecessary from reading the consumer binding, placing those properties into the consuming node.
* Downstream DTs have more eFuse fields declared than the one I use in this series [1]; they are also inconsistent in prefixing them efuse_ vs. otp_; in the RTD1295 datasheet the block is called eFuse, so I used efuse_ for consistency. I have enforced the dashes convention for nodes, as I didn't see the node name get used anywhere.
[1] https://patchwork.kernel.org/patch/11619643/
One more comment inline...
>> v2: New >> MAINTAINERS | 1 + >> drivers/nvmem/Kconfig | 9 ++++ >> drivers/nvmem/Makefile | 2 + >> drivers/nvmem/rtk-dhc-efuse.c | 86 +++++++++++++++++++++++++++++++++++ >> 4 files changed, 98 insertions(+) >> create mode 100644 drivers/nvmem/rtk-dhc-efuse.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 1d0d6ab20451..02117fbf0e57 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -2312,6 +2312,7 @@ F: >> Documentation/devicetree/bindings/soc/realtek/ >> F: arch/arm/boot/dts/rtd* >> F: arch/arm/mach-realtek/ >> F: arch/arm64/boot/dts/realtek/ >> +F: drivers/nvmem/rtk-dhc-efuse.c >> F: drivers/soc/realtek/ >> ARM/RENESAS ARM64 ARCHITECTURE [snip]
This line addition will conflict with the next line, added earlier in this patchset. Same for the binding patch. Do you need a v3 reordering them? This driver seems easier to target for 5.9 than the rest of the series.
If you do not intend to take the dt-bindings patch (17/29) through your tree, I can queue it once ack'ed by Rob and you.
Thanks for the quick review,
Andreas
-- SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
| |