| Message ID | 20240821105943.230281-1-ada@thorsis.com |
|---|---|
| Headers | show |
| Series | Microchip OTPC driver on SAM9X60 exposing UIDxR as additional nvmem device | expand |
On 21.08.2024 13:59, Alexander Dahl wrote: > See datasheet (DS60001579G) sections "7. Memories" and "23. OTP Memory > Controller (OTPC)" for reference. Please detail here what the patch does and why it is necessary. Sending the reader to some DS chapters w/o any additional information may be worthless. > > Signed-off-by: Alexander Dahl <ada@thorsis.com> > --- > arch/arm/boot/dts/microchip/sam9x60.dtsi | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi > index 291540e5d81e..2159a6817f44 100644 > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > @@ -14,6 +14,7 @@ > #include <dt-bindings/clock/at91.h> > #include <dt-bindings/mfd/at91-usart.h> > #include <dt-bindings/mfd/atmel-flexcom.h> > +#include <dt-bindings/nvmem/microchip,sama7g5-otpc.h> > > / { > #address-cells = <1>; > @@ -156,6 +157,13 @@ sdmmc1: sdio-host@90000000 { > status = "disabled"; > }; > > + otpc: efuse@eff00000 { > + compatible = "microchip,sam9x60-otpc", "syscon"; > + reg = <0xeff00000 0xec>; > + #address-cells = <1>; > + #size-cells = <1>; > + }; > + > apb { > compatible = "simple-bus"; > #address-cells = <1>;
On 21.08.2024 13:59, Alexander Dahl wrote: > This will allow to enable the main RC Oscillator from > nvmem_microchip_otpc driver. > > Signed-off-by: Alexander Dahl <ada@thorsis.com> > --- > > Notes: > This requires some DT bindings update, right? Yes > > arch/arm/boot/dts/microchip/sam9x60.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi > index 2159a6817f44..4f0651b8cb60 100644 > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > @@ -162,6 +162,8 @@ otpc: efuse@eff00000 { > reg = <0xeff00000 0xec>; > #address-cells = <1>; > #size-cells = <1>; > + clocks = <&pmc PMC_TYPE_CORE 11>; > + clock-names = "main_rc_osc"; And this should be squashed w/ patch 05/12. > }; > > apb {
Hi, Alexander, On 21.08.2024 13:59, Alexander Dahl wrote: > Hei hei, > > on a custom sam9x60 based board we want to access a unique ID of the > SoC. Microchip sam-ba has a command 'readuniqueid' which returns the > content of the OTPC Product UID x Register in that case. > > (On different boards with a SAMA5D2 we use the Serial Number x Register > exposed through the atmel soc driver. Those registers are not present > in the SAM9X60 series, but only for SAMA5D2/SAMA5D4 AFAIK.) Not sure if you are talking about Chip ID, Chip ID extension registers. These are available also on SAM9X60. > > There is a driver for the OTPC of the SAMA7G5 and after comparing > register layouts it seems that one is almost identical to the one used > by SAM9X60. Currently that driver has no support for the UIDx > registers, but I suppose it would be the right place to implement it, > because the registers are within the OTPC register address offsets. > > The patch series starts with fixups for the current driver. It then > adds the necessary pieces to DT and driver to work on SAM9X60 in > general. Later support for enabling the main RC oscillator is added, > which is required on SAM9X60 for the OTPC to work. The last patch adds > an additional nvmem device for the UIDx registers. > > This v1 of the series was _not_ tested on SAMA7G5, because I don't have > such a board for testing. Actually I don't know if the main_rc_osc > clock is required on SAMA7G5 too, and if yes how to handle that with > regard to the different clock ids. If someone could test on SAMA7G5 > and/or help me sorting out the core clock id things, that would be > highly appreciated. Please add Nicolas in the loop on the next revisions of this series as this should also be tested on SAMA7G5. I don't have a SAMA7G5 with OTP memory populated. > > Also I assume some more devicetree and/or sysfs documentation is > necessary. If someone could point me what's exactly required, this > would be very helpful for me. You see I expect at least another version > v2 of the series. ;-) > > Maybe some files having that "sama7g5" should be renamed, because that > DT binding is used for more SoCs now and deserves a more generic name? Not needed, adding your compatible there is enough. > Thinking of these for example: > > - Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml > - include/dt-bindings/nvmem/microchip,sama7g5-otpc.h > > Are there other SoCs than SAMA7G5 and SAM9X60 using the same OTPC? > > Last question: Should the UID be added to the device entropy pool with > add_device_randomness() as done in the SAMA5D2 sfr driver? > > I sent an RFC patch on this topic earlier this year, you'll find the > link below as a reference to the discussion. The patch itself was > trivial and not meant for applying as is anyways, so I decided to not > write a full changelog from RFC to v1. > > Last not least, special thanks to Christian Melki on IRC, who wrote and > tested parts of this, and was very kind and helpful in discussing the > topic several times in the past months. > > Christian, if you feel there's credit missing, just point me where to > add Co-developed-by and I'll happily do that for v2. > > Greets > Alex > > (series based on v6.11-rc4) > > Cc: linux-arm-kernel@lists.infradead.org > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-clk@vger.kernel.org > Link: https://lore.kernel.org/all/20240412140802.1571935-2-ada@thorsis.com/ > > Alexander Dahl (12): > nvmem: microchip-otpc: Avoid writing a write-only register > nvmem: microchip-otpc: Fix swapped 'sleep' and 'timeout' parameters > dt-bindings: nvmem: microchip-otpc: Add compatible for SAM9X60 > nvmem: microchip-otpc: Add SAM9X60 support > ARM: dts: microchip: sam9x60: Add OTPC node > ARM: dts: microchip: sam9x60_curiosity: Enable OTP Controller > nvmem: microchip-otpc: Add missing register definitions > nvmem: microchip-otpc: Add warnings for bad OTPC conditions on probe > clk: at91: sam9x60: Allow enabling main_rc_osc through DT > ARM: dts: microchip: sam9x60: Add clock properties to OTPC > nvmem: microchip-otpc: Enable main RC oscillator clock > nvmem: microchip-otpc: Expose UID registers as 2nd nvmem device > > .../nvmem/microchip,sama7g5-otpc.yaml | 1 + > .../dts/microchip/at91-sam9x60_curiosity.dts | 4 + > arch/arm/boot/dts/microchip/sam9x60.dtsi | 10 +++ > drivers/clk/at91/sam9x60.c | 3 +- > drivers/nvmem/microchip-otpc.c | 86 ++++++++++++++++++- > include/dt-bindings/clock/at91.h | 1 + > 6 files changed, 100 insertions(+), 5 deletions(-) > > > base-commit: 47ac09b91befbb6a235ab620c32af719f8208399
Hello Claudiu, thanks for having a closer look on the series. The issues the bots complained about are already fixed in my working copy and will be part of v2. Detailed discussion on particular patches itself over there, general remarks below. Am Sat, Aug 24, 2024 at 07:17:43PM +0300 schrieb claudiu beznea: > Hi, Alexander, > > On 21.08.2024 13:59, Alexander Dahl wrote: > > Hei hei, > > > > on a custom sam9x60 based board we want to access a unique ID of the > > SoC. Microchip sam-ba has a command 'readuniqueid' which returns the > > content of the OTPC Product UID x Register in that case. > > > > (On different boards with a SAMA5D2 we use the Serial Number x Register > > exposed through the atmel soc driver. Those registers are not present > > in the SAM9X60 series, but only for SAMA5D2/SAMA5D4 AFAIK.) > > Not sure if you are talking about Chip ID, Chip ID extension registers. > These are available also on SAM9X60. No, this is not what I'm talking about. The Chip ID and Chip ID extension registers are common over all SoCs of the same type. What I need is a unique ID, the same sam-ba returns with the "readuniqueid" applet. The SAMA5D2 has this in SFR_SN0 and SFR_SN1, handled by drivers/soc/atmel/sfr.c driver. The SFR block on sam9x60 has no SNx registers, the unique ID comes from OTPC_UIDxR here. Best thing would be a simple nvmem device for the SAM9X60 providing just those 4 registers, in a similar way the sfr driver does for SAMA5D2. This is the motivation for the series and what's eventually done in patch 12. The other patches are just fixing the otpc driver for SAM9X60 so I can add that nvmem stuff. Greets Alex > > There is a driver for the OTPC of the SAMA7G5 and after comparing > > register layouts it seems that one is almost identical to the one used > > by SAM9X60. Currently that driver has no support for the UIDx > > registers, but I suppose it would be the right place to implement it, > > because the registers are within the OTPC register address offsets. > > > > The patch series starts with fixups for the current driver. It then > > adds the necessary pieces to DT and driver to work on SAM9X60 in > > general. Later support for enabling the main RC oscillator is added, > > which is required on SAM9X60 for the OTPC to work. The last patch adds > > an additional nvmem device for the UIDx registers. > > > > This v1 of the series was _not_ tested on SAMA7G5, because I don't have > > such a board for testing. Actually I don't know if the main_rc_osc > > clock is required on SAMA7G5 too, and if yes how to handle that with > > regard to the different clock ids. If someone could test on SAMA7G5 > > and/or help me sorting out the core clock id things, that would be > > highly appreciated. > > Please add Nicolas in the loop on the next revisions of this series as this > should also be tested on SAMA7G5. I don't have a SAMA7G5 with OTP memory > populated. > > > > > Also I assume some more devicetree and/or sysfs documentation is > > necessary. If someone could point me what's exactly required, this > > would be very helpful for me. You see I expect at least another version > > v2 of the series. ;-) > > > > Maybe some files having that "sama7g5" should be renamed, because that > > DT binding is used for more SoCs now and deserves a more generic name? > > Not needed, adding your compatible there is enough. > > > Thinking of these for example: > > > > - Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml > > - include/dt-bindings/nvmem/microchip,sama7g5-otpc.h > > > > Are there other SoCs than SAMA7G5 and SAM9X60 using the same OTPC? > > > > Last question: Should the UID be added to the device entropy pool with > > add_device_randomness() as done in the SAMA5D2 sfr driver? > > > > I sent an RFC patch on this topic earlier this year, you'll find the > > link below as a reference to the discussion. The patch itself was > > trivial and not meant for applying as is anyways, so I decided to not > > write a full changelog from RFC to v1. > > > > Last not least, special thanks to Christian Melki on IRC, who wrote and > > tested parts of this, and was very kind and helpful in discussing the > > topic several times in the past months. > > > > Christian, if you feel there's credit missing, just point me where to > > add Co-developed-by and I'll happily do that for v2. > > > > Greets > > Alex > > > > (series based on v6.11-rc4) > > > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-clk@vger.kernel.org > > Link: https://lore.kernel.org/all/20240412140802.1571935-2-ada@thorsis.com/ > > > > Alexander Dahl (12): > > nvmem: microchip-otpc: Avoid writing a write-only register > > nvmem: microchip-otpc: Fix swapped 'sleep' and 'timeout' parameters > > dt-bindings: nvmem: microchip-otpc: Add compatible for SAM9X60 > > nvmem: microchip-otpc: Add SAM9X60 support > > ARM: dts: microchip: sam9x60: Add OTPC node > > ARM: dts: microchip: sam9x60_curiosity: Enable OTP Controller > > nvmem: microchip-otpc: Add missing register definitions > > nvmem: microchip-otpc: Add warnings for bad OTPC conditions on probe > > clk: at91: sam9x60: Allow enabling main_rc_osc through DT > > ARM: dts: microchip: sam9x60: Add clock properties to OTPC > > nvmem: microchip-otpc: Enable main RC oscillator clock > > nvmem: microchip-otpc: Expose UID registers as 2nd nvmem device > > > > .../nvmem/microchip,sama7g5-otpc.yaml | 1 + > > .../dts/microchip/at91-sam9x60_curiosity.dts | 4 + > > arch/arm/boot/dts/microchip/sam9x60.dtsi | 10 +++ > > drivers/clk/at91/sam9x60.c | 3 +- > > drivers/nvmem/microchip-otpc.c | 86 ++++++++++++++++++- > > include/dt-bindings/clock/at91.h | 1 + > > 6 files changed, 100 insertions(+), 5 deletions(-) > > > > > > base-commit: 47ac09b91befbb6a235ab620c32af719f8208399
Hello Claudiu, Am Sat, Aug 24, 2024 at 06:57:23PM +0300 schrieb claudiu beznea: > > > On 21.08.2024 13:59, Alexander Dahl wrote: > > This will allow to enable the main RC Oscillator from > > nvmem_microchip_otpc driver. > > > > Signed-off-by: Alexander Dahl <ada@thorsis.com> > > --- > > > > Notes: > > This requires some DT bindings update, right? > > Yes Okay, I suspected that already. This raises the question if the main rc osc clock should be mandatory for SAMA7G5 too, or if SAM9X60 and SAMA7G5 should be handled differently with regard to that clock? Affects both binding and driver then, see NOTE in patch 11, which is directly related to that question. > > > > > arch/arm/boot/dts/microchip/sam9x60.dtsi | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi > > index 2159a6817f44..4f0651b8cb60 100644 > > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > > @@ -162,6 +162,8 @@ otpc: efuse@eff00000 { > > reg = <0xeff00000 0xec>; > > #address-cells = <1>; > > #size-cells = <1>; > > + clocks = <&pmc PMC_TYPE_CORE 11>; > > + clock-names = "main_rc_osc"; > > And this should be squashed w/ patch 05/12. Okay, this may require patch reordering, we'll see. Greets Alex > > > }; > > > > apb {
Hi, Alexander, On 28.08.2024 10:31, Alexander Dahl wrote: > Hello Claudiu, > > thanks for having a closer look on the series. The issues the bots > complained about are already fixed in my working copy and will be part > of v2. Detailed discussion on particular patches itself over there, > general remarks below. > > Am Sat, Aug 24, 2024 at 07:17:43PM +0300 schrieb claudiu beznea: >> Hi, Alexander, >> >> On 21.08.2024 13:59, Alexander Dahl wrote: >>> Hei hei, >>> >>> on a custom sam9x60 based board we want to access a unique ID of the >>> SoC. Microchip sam-ba has a command 'readuniqueid' which returns the >>> content of the OTPC Product UID x Register in that case. >>> >>> (On different boards with a SAMA5D2 we use the Serial Number x Register >>> exposed through the atmel soc driver. Those registers are not present >>> in the SAM9X60 series, but only for SAMA5D2/SAMA5D4 AFAIK.) >> >> Not sure if you are talking about Chip ID, Chip ID extension registers. >> These are available also on SAM9X60. > > No, this is not what I'm talking about. The Chip ID and Chip ID > extension registers are common over all SoCs of the same type. > > What I need is a unique ID, the same sam-ba returns with the > "readuniqueid" applet. The SAMA5D2 has this in SFR_SN0 and SFR_SN1, > handled by drivers/soc/atmel/sfr.c driver. The SFR block on sam9x60 I see, I missed this one. > has no SNx registers, the unique ID comes from OTPC_UIDxR here. > > Best thing would be a simple nvmem device for the SAM9X60 providing > just those 4 registers, in a similar way the sfr driver does for > SAMA5D2. This is the motivation for the series and what's eventually > done in patch 12. The other patches are just fixing the otpc driver > for SAM9X60 so I can add that nvmem stuff. Got it, thanks for details. Thank you, Claudiu Beznea > > Greets > Alex > >>> There is a driver for the OTPC of the SAMA7G5 and after comparing >>> register layouts it seems that one is almost identical to the one used >>> by SAM9X60. Currently that driver has no support for the UIDx >>> registers, but I suppose it would be the right place to implement it, >>> because the registers are within the OTPC register address offsets. >>> >>> The patch series starts with fixups for the current driver. It then >>> adds the necessary pieces to DT and driver to work on SAM9X60 in >>> general. Later support for enabling the main RC oscillator is added, >>> which is required on SAM9X60 for the OTPC to work. The last patch adds >>> an additional nvmem device for the UIDx registers. >>> >>> This v1 of the series was _not_ tested on SAMA7G5, because I don't have >>> such a board for testing. Actually I don't know if the main_rc_osc >>> clock is required on SAMA7G5 too, and if yes how to handle that with >>> regard to the different clock ids. If someone could test on SAMA7G5 >>> and/or help me sorting out the core clock id things, that would be >>> highly appreciated. >> >> Please add Nicolas in the loop on the next revisions of this series as this >> should also be tested on SAMA7G5. I don't have a SAMA7G5 with OTP memory >> populated. >> >>> >>> Also I assume some more devicetree and/or sysfs documentation is >>> necessary. If someone could point me what's exactly required, this >>> would be very helpful for me. You see I expect at least another version >>> v2 of the series. ;-) >>> >>> Maybe some files having that "sama7g5" should be renamed, because that >>> DT binding is used for more SoCs now and deserves a more generic name? >> >> Not needed, adding your compatible there is enough. >> >>> Thinking of these for example: >>> >>> - Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml >>> - include/dt-bindings/nvmem/microchip,sama7g5-otpc.h >>> >>> Are there other SoCs than SAMA7G5 and SAM9X60 using the same OTPC? >>> >>> Last question: Should the UID be added to the device entropy pool with >>> add_device_randomness() as done in the SAMA5D2 sfr driver? >>> >>> I sent an RFC patch on this topic earlier this year, you'll find the >>> link below as a reference to the discussion. The patch itself was >>> trivial and not meant for applying as is anyways, so I decided to not >>> write a full changelog from RFC to v1. >>> >>> Last not least, special thanks to Christian Melki on IRC, who wrote and >>> tested parts of this, and was very kind and helpful in discussing the >>> topic several times in the past months. >>> >>> Christian, if you feel there's credit missing, just point me where to >>> add Co-developed-by and I'll happily do that for v2. >>> >>> Greets >>> Alex >>> >>> (series based on v6.11-rc4) >>> >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: devicetree@vger.kernel.org >>> Cc: linux-kernel@vger.kernel.org >>> Cc: linux-clk@vger.kernel.org >>> Link: https://lore.kernel.org/all/20240412140802.1571935-2-ada@thorsis.com/ >>> >>> Alexander Dahl (12): >>> nvmem: microchip-otpc: Avoid writing a write-only register >>> nvmem: microchip-otpc: Fix swapped 'sleep' and 'timeout' parameters >>> dt-bindings: nvmem: microchip-otpc: Add compatible for SAM9X60 >>> nvmem: microchip-otpc: Add SAM9X60 support >>> ARM: dts: microchip: sam9x60: Add OTPC node >>> ARM: dts: microchip: sam9x60_curiosity: Enable OTP Controller >>> nvmem: microchip-otpc: Add missing register definitions >>> nvmem: microchip-otpc: Add warnings for bad OTPC conditions on probe >>> clk: at91: sam9x60: Allow enabling main_rc_osc through DT >>> ARM: dts: microchip: sam9x60: Add clock properties to OTPC >>> nvmem: microchip-otpc: Enable main RC oscillator clock >>> nvmem: microchip-otpc: Expose UID registers as 2nd nvmem device >>> >>> .../nvmem/microchip,sama7g5-otpc.yaml | 1 + >>> .../dts/microchip/at91-sam9x60_curiosity.dts | 4 + >>> arch/arm/boot/dts/microchip/sam9x60.dtsi | 10 +++ >>> drivers/clk/at91/sam9x60.c | 3 +- >>> drivers/nvmem/microchip-otpc.c | 86 ++++++++++++++++++- >>> include/dt-bindings/clock/at91.h | 1 + >>> 6 files changed, 100 insertions(+), 5 deletions(-) >>> >>> >>> base-commit: 47ac09b91befbb6a235ab620c32af719f8208399