diff mbox series

[v1,07/12] nvmem: microchip-otpc: Add missing register definitions

Message ID 20240821105943.230281-8-ada@thorsis.com (mailing list archive)
State New, archived
Headers show
Series Microchip OTPC driver on SAM9X60 exposing UIDxR as additional nvmem device | expand

Commit Message

Alexander Dahl Aug. 21, 2024, 10:59 a.m. UTC 
According to datasheets DS60001765B for SAMA7G5 and DS60001579G for
SAM9X60.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/nvmem/microchip-otpc.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Claudiu Beznea Aug. 24, 2024, 3:54 p.m. UTC | #1 
On 21.08.2024 13:59, Alexander Dahl wrote:
> According to datasheets DS60001765B for SAMA7G5 and DS60001579G for
> SAM9X60.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
>  drivers/nvmem/microchip-otpc.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> index b8ed7412dbca..4630e96243ac 100644
> --- a/drivers/nvmem/microchip-otpc.c
> +++ b/drivers/nvmem/microchip-otpc.c
> @@ -21,9 +21,24 @@
>  #define MCHP_OTPC_AR			(0x8)
>  #define MCHP_OTPC_SR			(0xc)
>  #define MCHP_OTPC_SR_READ		BIT(6)
> +#define MCHP_OTPC_IER			(0x10)
> +#define MCHP_OTPC_IDR			(0x14)
> +#define MCHP_OTPC_IMR			(0x18)
> +#define MCHP_OTPC_ISR			(0x1C)
> +#define MCHP_OTPC_ISR_COERR		BIT(13)
>  #define MCHP_OTPC_HR			(0x20)
>  #define MCHP_OTPC_HR_SIZE		GENMASK(15, 8)
>  #define MCHP_OTPC_DR			(0x24)
> +#define MCHP_OTPC_BAR			(0x30)
> +#define MCHP_OTPC_CAR			(0x34)
> +#define MCHP_OTPC_UHC0R			(0x50)
> +#define MCHP_OTPC_UHC1R			(0x54)
> +#define MCHP_OTPC_UID0R			(0x60)
> +#define MCHP_OTPC_UID1R			(0x64)
> +#define MCHP_OTPC_UID2R			(0x68)
> +#define MCHP_OTPC_UID3R			(0x6C)
> +#define MCHP_OTPC_WPMR			(0xE4)
> +#define MCHP_OTPC_WPSR			(0xE8)

Are all these used in driver?

>  
>  #define MCHP_OTPC_NAME			"mchp-otpc"
>  #define MCHP_OTPC_SIZE			(11 * 1024)
Alexander Dahl Aug. 28, 2024, 8:14 a.m. UTC | #2 
Hello Claudiu,

Am Sat, Aug 24, 2024 at 06:54:02PM +0300 schrieb claudiu beznea:
> 
> 
> On 21.08.2024 13:59, Alexander Dahl wrote:
> > According to datasheets DS60001765B for SAMA7G5 and DS60001579G for
> > SAM9X60.
> > 
> > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > ---
> >  drivers/nvmem/microchip-otpc.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> > index b8ed7412dbca..4630e96243ac 100644
> > --- a/drivers/nvmem/microchip-otpc.c
> > +++ b/drivers/nvmem/microchip-otpc.c
> > @@ -21,9 +21,24 @@
> >  #define MCHP_OTPC_AR			(0x8)
> >  #define MCHP_OTPC_SR			(0xc)
> >  #define MCHP_OTPC_SR_READ		BIT(6)
> > +#define MCHP_OTPC_IER			(0x10)
> > +#define MCHP_OTPC_IDR			(0x14)
> > +#define MCHP_OTPC_IMR			(0x18)
> > +#define MCHP_OTPC_ISR			(0x1C)
> > +#define MCHP_OTPC_ISR_COERR		BIT(13)
> >  #define MCHP_OTPC_HR			(0x20)
> >  #define MCHP_OTPC_HR_SIZE		GENMASK(15, 8)
> >  #define MCHP_OTPC_DR			(0x24)
> > +#define MCHP_OTPC_BAR			(0x30)
> > +#define MCHP_OTPC_CAR			(0x34)
> > +#define MCHP_OTPC_UHC0R			(0x50)
> > +#define MCHP_OTPC_UHC1R			(0x54)
> > +#define MCHP_OTPC_UID0R			(0x60)
> > +#define MCHP_OTPC_UID1R			(0x64)
> > +#define MCHP_OTPC_UID2R			(0x68)
> > +#define MCHP_OTPC_UID3R			(0x6C)
> > +#define MCHP_OTPC_WPMR			(0xE4)
> > +#define MCHP_OTPC_WPSR			(0xE8)
> 
> Are all these used in driver?

Not all, but some.  What are you implying?  Only add register
definitions actually used in the driver?  Why?

Those register offsets won't change, but helped us when debugging.
Debug code (e.g. register dump) is not part of the patch series.

Greets
Alex

> 
> >  
> >  #define MCHP_OTPC_NAME			"mchp-otpc"
> >  #define MCHP_OTPC_SIZE			(11 * 1024)
Claudiu Beznea Aug. 31, 2024, 3:33 p.m. UTC | #3 
On 28.08.2024 11:14, Alexander Dahl wrote:
> Hello Claudiu,
> 
> Am Sat, Aug 24, 2024 at 06:54:02PM +0300 schrieb claudiu beznea:
>>
>>
>> On 21.08.2024 13:59, Alexander Dahl wrote:
>>> According to datasheets DS60001765B for SAMA7G5 and DS60001579G for
>>> SAM9X60.
>>>
>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>> ---
>>>  drivers/nvmem/microchip-otpc.c | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
>>> index b8ed7412dbca..4630e96243ac 100644
>>> --- a/drivers/nvmem/microchip-otpc.c
>>> +++ b/drivers/nvmem/microchip-otpc.c
>>> @@ -21,9 +21,24 @@
>>>  #define MCHP_OTPC_AR			(0x8)
>>>  #define MCHP_OTPC_SR			(0xc)
>>>  #define MCHP_OTPC_SR_READ		BIT(6)
>>> +#define MCHP_OTPC_IER			(0x10)
>>> +#define MCHP_OTPC_IDR			(0x14)
>>> +#define MCHP_OTPC_IMR			(0x18)
>>> +#define MCHP_OTPC_ISR			(0x1C)
>>> +#define MCHP_OTPC_ISR_COERR		BIT(13)
>>>  #define MCHP_OTPC_HR			(0x20)
>>>  #define MCHP_OTPC_HR_SIZE		GENMASK(15, 8)
>>>  #define MCHP_OTPC_DR			(0x24)
>>> +#define MCHP_OTPC_BAR			(0x30)
>>> +#define MCHP_OTPC_CAR			(0x34)
>>> +#define MCHP_OTPC_UHC0R			(0x50)
>>> +#define MCHP_OTPC_UHC1R			(0x54)
>>> +#define MCHP_OTPC_UID0R			(0x60)
>>> +#define MCHP_OTPC_UID1R			(0x64)
>>> +#define MCHP_OTPC_UID2R			(0x68)
>>> +#define MCHP_OTPC_UID3R			(0x6C)
>>> +#define MCHP_OTPC_WPMR			(0xE4)
>>> +#define MCHP_OTPC_WPSR			(0xE8)
>>
>> Are all these used in driver?
> 
> Not all, but some.  What are you implying?  Only add register
> definitions actually used in the driver? 

Yes!

> Why?

Less code to maintain. If it's not used there is no meaning to have it.


> 
> Those register offsets won't change, but helped us when debugging.
> Debug code (e.g. register dump) is not part of the patch series.
> 
> Greets
> Alex
> 
>>
>>>  
>>>  #define MCHP_OTPC_NAME			"mchp-otpc"
>>>  #define MCHP_OTPC_SIZE			(11 * 1024)
Alexander Dahl Sept. 2, 2024, 8:08 a.m. UTC | #4 
Hello Claudiu,

Am Sat, Aug 31, 2024 at 06:33:50PM +0300 schrieb claudiu beznea:
> 
> 
> On 28.08.2024 11:14, Alexander Dahl wrote:
> > Hello Claudiu,
> > 
> > Am Sat, Aug 24, 2024 at 06:54:02PM +0300 schrieb claudiu beznea:
> >>
> >>
> >> On 21.08.2024 13:59, Alexander Dahl wrote:
> >>> According to datasheets DS60001765B for SAMA7G5 and DS60001579G for
> >>> SAM9X60.
> >>>
> >>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> >>> ---
> >>>  drivers/nvmem/microchip-otpc.c | 15 +++++++++++++++
> >>>  1 file changed, 15 insertions(+)
> >>>
> >>> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> >>> index b8ed7412dbca..4630e96243ac 100644
> >>> --- a/drivers/nvmem/microchip-otpc.c
> >>> +++ b/drivers/nvmem/microchip-otpc.c
> >>> @@ -21,9 +21,24 @@
> >>>  #define MCHP_OTPC_AR			(0x8)
> >>>  #define MCHP_OTPC_SR			(0xc)
> >>>  #define MCHP_OTPC_SR_READ		BIT(6)
> >>> +#define MCHP_OTPC_IER			(0x10)
> >>> +#define MCHP_OTPC_IDR			(0x14)
> >>> +#define MCHP_OTPC_IMR			(0x18)
> >>> +#define MCHP_OTPC_ISR			(0x1C)
> >>> +#define MCHP_OTPC_ISR_COERR		BIT(13)
> >>>  #define MCHP_OTPC_HR			(0x20)
> >>>  #define MCHP_OTPC_HR_SIZE		GENMASK(15, 8)
> >>>  #define MCHP_OTPC_DR			(0x24)
> >>> +#define MCHP_OTPC_BAR			(0x30)
> >>> +#define MCHP_OTPC_CAR			(0x34)
> >>> +#define MCHP_OTPC_UHC0R			(0x50)
> >>> +#define MCHP_OTPC_UHC1R			(0x54)
> >>> +#define MCHP_OTPC_UID0R			(0x60)
> >>> +#define MCHP_OTPC_UID1R			(0x64)
> >>> +#define MCHP_OTPC_UID2R			(0x68)
> >>> +#define MCHP_OTPC_UID3R			(0x6C)
> >>> +#define MCHP_OTPC_WPMR			(0xE4)
> >>> +#define MCHP_OTPC_WPSR			(0xE8)
> >>
> >> Are all these used in driver?
> > 
> > Not all, but some.  What are you implying?  Only add register
> > definitions actually used in the driver? 
> 
> Yes!

Okay.

So if I drop the patch with the warnings on driver probe you did not
like (checking for pre probe error conditions in interrupt register
for example), then it is just the MCHP_OTPC_UID0R and I would squash
that one definition in the last patch adding the nvmem for the UID
then, okay?

Greets
Alex

> 
> > Why?
> 
> Less code to maintain. If it's not used there is no meaning to have it.
> 
> 
> > 
> > Those register offsets won't change, but helped us when debugging.
> > Debug code (e.g. register dump) is not part of the patch series.
> > 
> > Greets
> > Alex
> > 
> >>
> >>>  
> >>>  #define MCHP_OTPC_NAME			"mchp-otpc"
> >>>  #define MCHP_OTPC_SIZE			(11 * 1024)
>
Claudiu Beznea Sept. 7, 2024, 12:05 p.m. UTC | #5 
On 02.09.2024 11:08, Alexander Dahl wrote:
> Hello Claudiu,
> 
> Am Sat, Aug 31, 2024 at 06:33:50PM +0300 schrieb claudiu beznea:
>>
>>
>> On 28.08.2024 11:14, Alexander Dahl wrote:
>>> Hello Claudiu,
>>>
>>> Am Sat, Aug 24, 2024 at 06:54:02PM +0300 schrieb claudiu beznea:
>>>>
>>>>
>>>> On 21.08.2024 13:59, Alexander Dahl wrote:
>>>>> According to datasheets DS60001765B for SAMA7G5 and DS60001579G for
>>>>> SAM9X60.
>>>>>
>>>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>>>> ---
>>>>>  drivers/nvmem/microchip-otpc.c | 15 +++++++++++++++
>>>>>  1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
>>>>> index b8ed7412dbca..4630e96243ac 100644
>>>>> --- a/drivers/nvmem/microchip-otpc.c
>>>>> +++ b/drivers/nvmem/microchip-otpc.c
>>>>> @@ -21,9 +21,24 @@
>>>>>  #define MCHP_OTPC_AR			(0x8)
>>>>>  #define MCHP_OTPC_SR			(0xc)
>>>>>  #define MCHP_OTPC_SR_READ		BIT(6)
>>>>> +#define MCHP_OTPC_IER			(0x10)
>>>>> +#define MCHP_OTPC_IDR			(0x14)
>>>>> +#define MCHP_OTPC_IMR			(0x18)
>>>>> +#define MCHP_OTPC_ISR			(0x1C)
>>>>> +#define MCHP_OTPC_ISR_COERR		BIT(13)
>>>>>  #define MCHP_OTPC_HR			(0x20)
>>>>>  #define MCHP_OTPC_HR_SIZE		GENMASK(15, 8)
>>>>>  #define MCHP_OTPC_DR			(0x24)
>>>>> +#define MCHP_OTPC_BAR			(0x30)
>>>>> +#define MCHP_OTPC_CAR			(0x34)
>>>>> +#define MCHP_OTPC_UHC0R			(0x50)
>>>>> +#define MCHP_OTPC_UHC1R			(0x54)
>>>>> +#define MCHP_OTPC_UID0R			(0x60)
>>>>> +#define MCHP_OTPC_UID1R			(0x64)
>>>>> +#define MCHP_OTPC_UID2R			(0x68)
>>>>> +#define MCHP_OTPC_UID3R			(0x6C)
>>>>> +#define MCHP_OTPC_WPMR			(0xE4)
>>>>> +#define MCHP_OTPC_WPSR			(0xE8)
>>>>
>>>> Are all these used in driver?
>>>
>>> Not all, but some.  What are you implying?  Only add register
>>> definitions actually used in the driver? 
>>
>> Yes!
> 
> Okay.
> 
> So if I drop the patch with the warnings on driver probe you did not
> like (checking for pre probe error conditions in interrupt register
> for example), then it is just the MCHP_OTPC_UID0R and I would squash
> that one definition in the last patch adding the nvmem for the UID
> then, okay?

OK for me.

The idea would be to keep just what is used.

Thank you,
Claudiu Beznea

> 
> Greets
> Alex
> 
>>
>>> Why?
>>
>> Less code to maintain. If it's not used there is no meaning to have it.
>>
>>
>>>
>>> Those register offsets won't change, but helped us when debugging.
>>> Debug code (e.g. register dump) is not part of the patch series.
>>>
>>> Greets
>>> Alex
>>>
>>>>
>>>>>  
>>>>>  #define MCHP_OTPC_NAME			"mchp-otpc"
>>>>>  #define MCHP_OTPC_SIZE			(11 * 1024)
>>
diff mbox series

Patch

diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
index b8ed7412dbca..4630e96243ac 100644
--- a/drivers/nvmem/microchip-otpc.c
+++ b/drivers/nvmem/microchip-otpc.c
@@ -21,9 +21,24 @@ 
 #define MCHP_OTPC_AR			(0x8)
 #define MCHP_OTPC_SR			(0xc)
 #define MCHP_OTPC_SR_READ		BIT(6)
+#define MCHP_OTPC_IER			(0x10)
+#define MCHP_OTPC_IDR			(0x14)
+#define MCHP_OTPC_IMR			(0x18)
+#define MCHP_OTPC_ISR			(0x1C)
+#define MCHP_OTPC_ISR_COERR		BIT(13)
 #define MCHP_OTPC_HR			(0x20)
 #define MCHP_OTPC_HR_SIZE		GENMASK(15, 8)
 #define MCHP_OTPC_DR			(0x24)
+#define MCHP_OTPC_BAR			(0x30)
+#define MCHP_OTPC_CAR			(0x34)
+#define MCHP_OTPC_UHC0R			(0x50)
+#define MCHP_OTPC_UHC1R			(0x54)
+#define MCHP_OTPC_UID0R			(0x60)
+#define MCHP_OTPC_UID1R			(0x64)
+#define MCHP_OTPC_UID2R			(0x68)
+#define MCHP_OTPC_UID3R			(0x6C)
+#define MCHP_OTPC_WPMR			(0xE4)
+#define MCHP_OTPC_WPSR			(0xE8)
 
 #define MCHP_OTPC_NAME			"mchp-otpc"
 #define MCHP_OTPC_SIZE			(11 * 1024)