Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-20 Thread PrasannaKumar Muralidharan
On 11 January 2018 at 20:38, Rob Herring  wrote:
> On Sat, Jan 6, 2018 at 6:43 AM, PrasannaKumar Muralidharan
>  wrote:
>> Hi Rob,
>>
>> On 4 January 2018 at 01:32, Rob Herring  wrote:
>>> On Thu, Dec 28, 2017 at 10:29:52PM +0100, Mathieu Malaterre wrote:
 From: PrasannaKumar Muralidharan 

 This patch brings support for the JZ4780 efuse. Currently it only expose
 a read only access to the entire 8K bits efuse memory.

 Tested-by: Mathieu Malaterre 
 Signed-off-by: PrasannaKumar Muralidharan 
 Signed-off-by: Mathieu Malaterre 
 ---
  .../ABI/testing/sysfs-driver-jz4780-efuse  |  16 ++
  .../bindings/nvmem/ingenic,jz4780-efuse.txt|  17 ++
>>>
>>> Please split bindings to separate patch.
>>>
  MAINTAINERS|   5 +
  arch/mips/boot/dts/ingenic/jz4780.dtsi |  40 ++-
>>>
>>> dts files should also be separate.
>>>
  drivers/nvmem/Kconfig  |  10 +
  drivers/nvmem/Makefile |   2 +
  drivers/nvmem/jz4780-efuse.c   | 305 
 +
  7 files changed, 383 insertions(+), 12 deletions(-)
  create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse
  create mode 100644 
 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
  create mode 100644 drivers/nvmem/jz4780-efuse.c

 diff --git a/Documentation/ABI/testing/sysfs-driver-jz4780-efuse 
 b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
 new file mode 100644
 index ..bb6f5d6ceea0
 --- /dev/null
 +++ b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
 @@ -0,0 +1,16 @@
 +What:/sys/devices/*//nvmem
 +Date:December 2017
 +Contact: PrasannaKumar Muralidharan 
 +Description: read-only access to the efuse on the Ingenic JZ4780 SoC
 + The SoC has a one time programmable 8K efuse that is
 + split into segments. The driver supports read only.
 + The segments are
 + 0x000   64 bit Random Number
 + 0x008  128 bit Ingenic Chip ID
 + 0x018  128 bit Customer ID
 + 0x028 3520 bit Reserved
 + 0x1E08 bit Protect Segment
 + 0x1E1 2296 bit HDMI Key
 + 0x300 2048 bit Security boot key
>>>
>>> Why do these need to be exposed to userspace?
>>>
>>> sysfs is 1 value per file and this is lots of different things.
>>>
>>> We already have ways to feed random data (entropy) to the system. And we
>>> have a way to expose SoC ID info to userspace (socdev).
>>
>> Currently ingenic chip id is not used anywhere. The vendor BSP exposed
>> only chip id and customer id. Should we do the same? Please provide
>> your suggestion.
>
> No. Don't create an ABI if you don't really need it.

Rob,
MAC address of the ethernet device is stored in customer id segment of
efuse. So only customer id is needed. Do you think exposing customer
id would suffice?

Srini,
Only user would be dm900 ethernet driver (need to make changes to it
once the efuse driver goes in). There is no need to expose it to user
space. I am planning to modify nvmem core to not expose efuse if the
efuse driver chooses so. Do you think it makes sense? The need to
maintain ABI for user space disappears if such a change is introduced.

Thanks,
PrasannaKumar


Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-17 Thread Rob Herring
On Wed, Jan 17, 2018 at 11:31 AM, PrasannaKumar Muralidharan
 wrote:
> Hi Rob,
>
> On 11 January 2018 at 20:38, Rob Herring  wrote:
>> On Sat, Jan 6, 2018 at 6:43 AM, PrasannaKumar Muralidharan
>>  wrote:
>>> Hi Rob,
>>>
>>> On 4 January 2018 at 01:32, Rob Herring  wrote:
 On Thu, Dec 28, 2017 at 10:29:52PM +0100, Mathieu Malaterre wrote:
> From: PrasannaKumar Muralidharan 

[...]

> + nemc: nemc@1341 {
> + compatible = "ingenic,jz4780-nemc";
> + reg = <0x1341 0x1>;
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges = <1 0 0x1b00 0x100
> +   2 0 0x1a00 0x100
> +   3 0 0x1900 0x100
> +   4 0 0x1800 0x100
> +   5 0 0x1700 0x100
> +   6 0 0x1600 0x100>;
> +
> + clocks = <&cgu JZ4780_CLK_NEMC>;
> +
> + status = "disabled";
> + };
>
> - clocks = <&cgu JZ4780_CLK_NEMC>;
> + efuse: efuse@134100d0 {
> + compatible = "ingenic,jz4780-efuse";
> + reg = <0x134100d0 0xff>;

 You are creating an overlapping region here with nemc above. Don't do
 that.
>>>
>>> Should "reg = <0x1341 0x1>;" be used instead?
>>
>> No, that still overlaps with nemc. What's in registers 0x00-0xcf and
>> 0x1d0-0x? Either get rid of this node altogether and make the
>> driver for nemc also instantiate the efuse driver (DT is not the only
>> way to instantiate drivers), or create sub-nodes under nemc for each
>> distinct h/w block in the 1341-1342 address space.
>
> My idea was not to change nemc driver.
>
> By "create sub-nodes under nemc" do you mean something like below?

Yes.

> nemc: nemc@1341 {
>  compatible = "ingenic,jz4780-nemc";
>  reg = <0x1341 0x1>;
>  <...>
>  status = "disabled";

Though having disabled here is strange. We'd normally ignore all the
child nodes.

>
>  efuse: efuse@134101d0 {
>   compatible = "ingenic,jz4780-efuse";
>   reg = <0x134100d0 0xff>;
>  }
> }
>
> Will this instantiate efuse driver? I do not know how to do that with
> sub-node. I will explore more on this.

The nemc driver just needs to call of_platform_default_populate.

>> Or a third option is make nemc reg:
>>
>> reg = <0x1341 0xd0>, <0x134101d0 0xfe30>;
>>
>> But I suspect that is wrong and you probably have some other function in 
>> there.
>>
>> Rob
>
> If the efuse block were to use a different base register address (that
> does not overlap with nemc register range) in future SoC how the node
> should be? Using nemc to instantiate efuse won't be the best choice if
> that happens.

Then you will have a different compatible for nemc (because it is
different) and then the driver should skip the above step.

Rob


Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-17 Thread PrasannaKumar Muralidharan
Hi Rob,

On 11 January 2018 at 20:38, Rob Herring  wrote:
> On Sat, Jan 6, 2018 at 6:43 AM, PrasannaKumar Muralidharan
>  wrote:
>> Hi Rob,
>>
>> On 4 January 2018 at 01:32, Rob Herring  wrote:
>>> On Thu, Dec 28, 2017 at 10:29:52PM +0100, Mathieu Malaterre wrote:
 From: PrasannaKumar Muralidharan 

 This patch brings support for the JZ4780 efuse. Currently it only expose
 a read only access to the entire 8K bits efuse memory.

 Tested-by: Mathieu Malaterre 
 Signed-off-by: PrasannaKumar Muralidharan 
 Signed-off-by: Mathieu Malaterre 
 ---
  .../ABI/testing/sysfs-driver-jz4780-efuse  |  16 ++
  .../bindings/nvmem/ingenic,jz4780-efuse.txt|  17 ++
>>>
>>> Please split bindings to separate patch.
>>>
  MAINTAINERS|   5 +
  arch/mips/boot/dts/ingenic/jz4780.dtsi |  40 ++-
>>>
>>> dts files should also be separate.
>>>
  drivers/nvmem/Kconfig  |  10 +
  drivers/nvmem/Makefile |   2 +
  drivers/nvmem/jz4780-efuse.c   | 305 
 +
  7 files changed, 383 insertions(+), 12 deletions(-)
  create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse
  create mode 100644 
 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
  create mode 100644 drivers/nvmem/jz4780-efuse.c

 diff --git a/Documentation/ABI/testing/sysfs-driver-jz4780-efuse 
 b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
 new file mode 100644
 index ..bb6f5d6ceea0
 --- /dev/null
 +++ b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
 @@ -0,0 +1,16 @@
 +What:/sys/devices/*//nvmem
 +Date:December 2017
 +Contact: PrasannaKumar Muralidharan 
 +Description: read-only access to the efuse on the Ingenic JZ4780 SoC
 + The SoC has a one time programmable 8K efuse that is
 + split into segments. The driver supports read only.
 + The segments are
 + 0x000   64 bit Random Number
 + 0x008  128 bit Ingenic Chip ID
 + 0x018  128 bit Customer ID
 + 0x028 3520 bit Reserved
 + 0x1E08 bit Protect Segment
 + 0x1E1 2296 bit HDMI Key
 + 0x300 2048 bit Security boot key
>>>
>>> Why do these need to be exposed to userspace?
>>>
>>> sysfs is 1 value per file and this is lots of different things.
>>>
>>> We already have ways to feed random data (entropy) to the system. And we
>>> have a way to expose SoC ID info to userspace (socdev).
>>
>> Currently ingenic chip id is not used anywhere. The vendor BSP exposed
>> only chip id and customer id. Should we do the same? Please provide
>> your suggestion.
>
> No. Don't create an ABI if you don't really need it.
>
>>
 +Users:   any user space application which wants to read the 
 Chip
 + and Customer ID
 diff --git 
 a/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt 
 b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
 new file mode 100644
 index ..cd6d67ec22fc
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
 @@ -0,0 +1,17 @@
 +Ingenic JZ EFUSE driver bindings
 +
 +Required properties:
 +- "compatible"   Must be set to "ingenic,jz4780-efuse"
 +- "reg"  Register location and length
 +- "clocks"   Handle for the ahb clock for the efuse.
 +- "clock-names"  Must be "bus_clk"
 +
 +Example:
 +
 +efuse: efuse@134100d0 {
 + compatible = "ingenic,jz4780-efuse";
 + reg = <0x134100D0 0xFF>;
 +
 + clocks = <&cgu JZ4780_CLK_AHB2>;
 + clock-names = "bus_clk";
 +};
 diff --git a/MAINTAINERS b/MAINTAINERS
 index a6e86e20761e..7a050c20c533 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -6902,6 +6902,11 @@ M: Zubair Lutfullah Kakakhel 
 
  S:   Maintained
  F:   drivers/dma/dma-jz4780.c

 +INGENIC JZ4780 EFUSE Driver
 +M:   PrasannaKumar Muralidharan 
 +S:   Maintained
 +F:   drivers/nvmem/jz4780-efuse.c
>>>
>>> Binding file?
>>
>> Sorry, missed it. Will add it.
>>
 +
  INGENIC JZ4780 NAND DRIVER
  M:   Harvey Hunt 
  L:   linux-...@lists.infradead.org
 diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi 
 b/arch/mips/boot/dts/ingenic/jz4780.dtsi
 index 9b5794667aee..3fb9d916a2ea 100644
 --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
 +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
 @@ -224,21 +224,37 @@
   reg = <0x10002000 0x100>;
   };

 - nemc: nemc@1341 {
 - compatible = "ingenic,jz478

Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-11 Thread Rob Herring
On Sat, Jan 6, 2018 at 6:43 AM, PrasannaKumar Muralidharan
 wrote:
> Hi Rob,
>
> On 4 January 2018 at 01:32, Rob Herring  wrote:
>> On Thu, Dec 28, 2017 at 10:29:52PM +0100, Mathieu Malaterre wrote:
>>> From: PrasannaKumar Muralidharan 
>>>
>>> This patch brings support for the JZ4780 efuse. Currently it only expose
>>> a read only access to the entire 8K bits efuse memory.
>>>
>>> Tested-by: Mathieu Malaterre 
>>> Signed-off-by: PrasannaKumar Muralidharan 
>>> Signed-off-by: Mathieu Malaterre 
>>> ---
>>>  .../ABI/testing/sysfs-driver-jz4780-efuse  |  16 ++
>>>  .../bindings/nvmem/ingenic,jz4780-efuse.txt|  17 ++
>>
>> Please split bindings to separate patch.
>>
>>>  MAINTAINERS|   5 +
>>>  arch/mips/boot/dts/ingenic/jz4780.dtsi |  40 ++-
>>
>> dts files should also be separate.
>>
>>>  drivers/nvmem/Kconfig  |  10 +
>>>  drivers/nvmem/Makefile |   2 +
>>>  drivers/nvmem/jz4780-efuse.c   | 305 
>>> +
>>>  7 files changed, 383 insertions(+), 12 deletions(-)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
>>>  create mode 100644 drivers/nvmem/jz4780-efuse.c
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-driver-jz4780-efuse 
>>> b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
>>> new file mode 100644
>>> index ..bb6f5d6ceea0
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
>>> @@ -0,0 +1,16 @@
>>> +What:/sys/devices/*//nvmem
>>> +Date:December 2017
>>> +Contact: PrasannaKumar Muralidharan 
>>> +Description: read-only access to the efuse on the Ingenic JZ4780 SoC
>>> + The SoC has a one time programmable 8K efuse that is
>>> + split into segments. The driver supports read only.
>>> + The segments are
>>> + 0x000   64 bit Random Number
>>> + 0x008  128 bit Ingenic Chip ID
>>> + 0x018  128 bit Customer ID
>>> + 0x028 3520 bit Reserved
>>> + 0x1E08 bit Protect Segment
>>> + 0x1E1 2296 bit HDMI Key
>>> + 0x300 2048 bit Security boot key
>>
>> Why do these need to be exposed to userspace?
>>
>> sysfs is 1 value per file and this is lots of different things.
>>
>> We already have ways to feed random data (entropy) to the system. And we
>> have a way to expose SoC ID info to userspace (socdev).
>
> Currently ingenic chip id is not used anywhere. The vendor BSP exposed
> only chip id and customer id. Should we do the same? Please provide
> your suggestion.

No. Don't create an ABI if you don't really need it.

>
>>> +Users:   any user space application which wants to read the 
>>> Chip
>>> + and Customer ID
>>> diff --git 
>>> a/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt 
>>> b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
>>> new file mode 100644
>>> index ..cd6d67ec22fc
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
>>> @@ -0,0 +1,17 @@
>>> +Ingenic JZ EFUSE driver bindings
>>> +
>>> +Required properties:
>>> +- "compatible"   Must be set to "ingenic,jz4780-efuse"
>>> +- "reg"  Register location and length
>>> +- "clocks"   Handle for the ahb clock for the efuse.
>>> +- "clock-names"  Must be "bus_clk"
>>> +
>>> +Example:
>>> +
>>> +efuse: efuse@134100d0 {
>>> + compatible = "ingenic,jz4780-efuse";
>>> + reg = <0x134100D0 0xFF>;
>>> +
>>> + clocks = <&cgu JZ4780_CLK_AHB2>;
>>> + clock-names = "bus_clk";
>>> +};
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index a6e86e20761e..7a050c20c533 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -6902,6 +6902,11 @@ M: Zubair Lutfullah Kakakhel 
>>> 
>>>  S:   Maintained
>>>  F:   drivers/dma/dma-jz4780.c
>>>
>>> +INGENIC JZ4780 EFUSE Driver
>>> +M:   PrasannaKumar Muralidharan 
>>> +S:   Maintained
>>> +F:   drivers/nvmem/jz4780-efuse.c
>>
>> Binding file?
>
> Sorry, missed it. Will add it.
>
>>> +
>>>  INGENIC JZ4780 NAND DRIVER
>>>  M:   Harvey Hunt 
>>>  L:   linux-...@lists.infradead.org
>>> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi 
>>> b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> index 9b5794667aee..3fb9d916a2ea 100644
>>> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> @@ -224,21 +224,37 @@
>>>   reg = <0x10002000 0x100>;
>>>   };
>>>
>>> - nemc: nemc@1341 {
>>> - compatible = "ingenic,jz4780-nemc";
>>> - reg = <0x1341 0x1>;
>>> - #address-cells = <2>;
>>> +
>>> + ahb2: ahb2 {
>>> + compatible = "simple-bus";
>>
>> This is an unrelat

Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-06 Thread PrasannaKumar Muralidharan
Hi Rob,

On 4 January 2018 at 01:32, Rob Herring  wrote:
> On Thu, Dec 28, 2017 at 10:29:52PM +0100, Mathieu Malaterre wrote:
>> From: PrasannaKumar Muralidharan 
>>
>> This patch brings support for the JZ4780 efuse. Currently it only expose
>> a read only access to the entire 8K bits efuse memory.
>>
>> Tested-by: Mathieu Malaterre 
>> Signed-off-by: PrasannaKumar Muralidharan 
>> Signed-off-by: Mathieu Malaterre 
>> ---
>>  .../ABI/testing/sysfs-driver-jz4780-efuse  |  16 ++
>>  .../bindings/nvmem/ingenic,jz4780-efuse.txt|  17 ++
>
> Please split bindings to separate patch.
>
>>  MAINTAINERS|   5 +
>>  arch/mips/boot/dts/ingenic/jz4780.dtsi |  40 ++-
>
> dts files should also be separate.
>
>>  drivers/nvmem/Kconfig  |  10 +
>>  drivers/nvmem/Makefile |   2 +
>>  drivers/nvmem/jz4780-efuse.c   | 305 
>> +
>>  7 files changed, 383 insertions(+), 12 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse
>>  create mode 100644 
>> Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
>>  create mode 100644 drivers/nvmem/jz4780-efuse.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-jz4780-efuse 
>> b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
>> new file mode 100644
>> index ..bb6f5d6ceea0
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
>> @@ -0,0 +1,16 @@
>> +What:/sys/devices/*//nvmem
>> +Date:December 2017
>> +Contact: PrasannaKumar Muralidharan 
>> +Description: read-only access to the efuse on the Ingenic JZ4780 SoC
>> + The SoC has a one time programmable 8K efuse that is
>> + split into segments. The driver supports read only.
>> + The segments are
>> + 0x000   64 bit Random Number
>> + 0x008  128 bit Ingenic Chip ID
>> + 0x018  128 bit Customer ID
>> + 0x028 3520 bit Reserved
>> + 0x1E08 bit Protect Segment
>> + 0x1E1 2296 bit HDMI Key
>> + 0x300 2048 bit Security boot key
>
> Why do these need to be exposed to userspace?
>
> sysfs is 1 value per file and this is lots of different things.
>
> We already have ways to feed random data (entropy) to the system. And we
> have a way to expose SoC ID info to userspace (socdev).

Currently ingenic chip id is not used anywhere. The vendor BSP exposed
only chip id and customer id. Should we do the same? Please provide
your suggestion.

>> +Users:   any user space application which wants to read the Chip
>> + and Customer ID
>> diff --git 
>> a/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt 
>> b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
>> new file mode 100644
>> index ..cd6d67ec22fc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
>> @@ -0,0 +1,17 @@
>> +Ingenic JZ EFUSE driver bindings
>> +
>> +Required properties:
>> +- "compatible"   Must be set to "ingenic,jz4780-efuse"
>> +- "reg"  Register location and length
>> +- "clocks"   Handle for the ahb clock for the efuse.
>> +- "clock-names"  Must be "bus_clk"
>> +
>> +Example:
>> +
>> +efuse: efuse@134100d0 {
>> + compatible = "ingenic,jz4780-efuse";
>> + reg = <0x134100D0 0xFF>;
>> +
>> + clocks = <&cgu JZ4780_CLK_AHB2>;
>> + clock-names = "bus_clk";
>> +};
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a6e86e20761e..7a050c20c533 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -6902,6 +6902,11 @@ M: Zubair Lutfullah Kakakhel 
>> 
>>  S:   Maintained
>>  F:   drivers/dma/dma-jz4780.c
>>
>> +INGENIC JZ4780 EFUSE Driver
>> +M:   PrasannaKumar Muralidharan 
>> +S:   Maintained
>> +F:   drivers/nvmem/jz4780-efuse.c
>
> Binding file?

Sorry, missed it. Will add it.

>> +
>>  INGENIC JZ4780 NAND DRIVER
>>  M:   Harvey Hunt 
>>  L:   linux-...@lists.infradead.org
>> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi 
>> b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> index 9b5794667aee..3fb9d916a2ea 100644
>> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> @@ -224,21 +224,37 @@
>>   reg = <0x10002000 0x100>;
>>   };
>>
>> - nemc: nemc@1341 {
>> - compatible = "ingenic,jz4780-nemc";
>> - reg = <0x1341 0x1>;
>> - #address-cells = <2>;
>> +
>> + ahb2: ahb2 {
>> + compatible = "simple-bus";
>
> This is an unrelated change and should be its own patch.

The efuse register address range is a subset of address range of nemc.
So decided to make nemc and efuse as nodes with parent node ahb2. This
is required for efuse driver to work. I am not able to understand what
you mean by u

Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-03 Thread Rob Herring
On Thu, Dec 28, 2017 at 10:29:52PM +0100, Mathieu Malaterre wrote:
> From: PrasannaKumar Muralidharan 
> 
> This patch brings support for the JZ4780 efuse. Currently it only expose
> a read only access to the entire 8K bits efuse memory.
> 
> Tested-by: Mathieu Malaterre 
> Signed-off-by: PrasannaKumar Muralidharan 
> Signed-off-by: Mathieu Malaterre 
> ---
>  .../ABI/testing/sysfs-driver-jz4780-efuse  |  16 ++
>  .../bindings/nvmem/ingenic,jz4780-efuse.txt|  17 ++

Please split bindings to separate patch.

>  MAINTAINERS|   5 +
>  arch/mips/boot/dts/ingenic/jz4780.dtsi |  40 ++-

dts files should also be separate.

>  drivers/nvmem/Kconfig  |  10 +
>  drivers/nvmem/Makefile |   2 +
>  drivers/nvmem/jz4780-efuse.c   | 305 
> +
>  7 files changed, 383 insertions(+), 12 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse
>  create mode 100644 
> Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
>  create mode 100644 drivers/nvmem/jz4780-efuse.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-jz4780-efuse 
> b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
> new file mode 100644
> index ..bb6f5d6ceea0
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-jz4780-efuse
> @@ -0,0 +1,16 @@
> +What:/sys/devices/*//nvmem
> +Date:December 2017
> +Contact: PrasannaKumar Muralidharan 
> +Description: read-only access to the efuse on the Ingenic JZ4780 SoC
> + The SoC has a one time programmable 8K efuse that is
> + split into segments. The driver supports read only.
> + The segments are
> + 0x000   64 bit Random Number
> + 0x008  128 bit Ingenic Chip ID
> + 0x018  128 bit Customer ID
> + 0x028 3520 bit Reserved
> + 0x1E08 bit Protect Segment
> + 0x1E1 2296 bit HDMI Key
> + 0x300 2048 bit Security boot key

Why do these need to be exposed to userspace?

sysfs is 1 value per file and this is lots of different things. 

We already have ways to feed random data (entropy) to the system. And we 
have a way to expose SoC ID info to userspace (socdev).

> +Users:   any user space application which wants to read the Chip
> + and Customer ID
> diff --git a/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt 
> b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
> new file mode 100644
> index ..cd6d67ec22fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
> @@ -0,0 +1,17 @@
> +Ingenic JZ EFUSE driver bindings
> +
> +Required properties:
> +- "compatible"   Must be set to "ingenic,jz4780-efuse"
> +- "reg"  Register location and length
> +- "clocks"   Handle for the ahb clock for the efuse.
> +- "clock-names"  Must be "bus_clk"
> +
> +Example:
> +
> +efuse: efuse@134100d0 {
> + compatible = "ingenic,jz4780-efuse";
> + reg = <0x134100D0 0xFF>;
> +
> + clocks = <&cgu JZ4780_CLK_AHB2>;
> + clock-names = "bus_clk";
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6e86e20761e..7a050c20c533 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6902,6 +6902,11 @@ M: Zubair Lutfullah Kakakhel 
> 
>  S:   Maintained
>  F:   drivers/dma/dma-jz4780.c
>  
> +INGENIC JZ4780 EFUSE Driver
> +M:   PrasannaKumar Muralidharan 
> +S:   Maintained
> +F:   drivers/nvmem/jz4780-efuse.c

Binding file?

> +
>  INGENIC JZ4780 NAND DRIVER
>  M:   Harvey Hunt 
>  L:   linux-...@lists.infradead.org
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi 
> b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index 9b5794667aee..3fb9d916a2ea 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -224,21 +224,37 @@
>   reg = <0x10002000 0x100>;
>   };
>  
> - nemc: nemc@1341 {
> - compatible = "ingenic,jz4780-nemc";
> - reg = <0x1341 0x1>;
> - #address-cells = <2>;
> +
> + ahb2: ahb2 {
> + compatible = "simple-bus";

This is an unrelated change and should be its own patch.

> + #address-cells = <1>;
>   #size-cells = <1>;
> - ranges = <1 0 0x1b00 0x100
> -   2 0 0x1a00 0x100
> -   3 0 0x1900 0x100
> -   4 0 0x1800 0x100
> -   5 0 0x1700 0x100
> -   6 0 0x1600 0x100>;
> + ranges = <>;
> +
> + nemc: nemc@1341 {
> + compatible = "ingenic,jz4780-nemc";
> + reg = <0x1341 0x1>;
> + #address-

Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-02 Thread PrasannaKumar Muralidharan
Hi Srini,

On 2 January 2018 at 17:32, Srinivas Kandagatla
 wrote:
>
>
> On 28/12/17 21:29, Mathieu Malaterre wrote:
>>
>> From: PrasannaKumar Muralidharan 
>>
>> This patch brings support for the JZ4780 efuse. Currently it only expose
>> a read only access to the entire 8K bits efuse memory.
>>
>> Tested-by: Mathieu Malaterre 
>> Signed-off-by: PrasannaKumar Muralidharan 
>> Signed-off-by: Mathieu Malaterre 
>> ---
>
> Please split this patch, as you are mixing code, documentation, dts and
> MAINTAINER changes here.
>
> Without which patch can not be reviewed!!

Sure, will do it soon.

>
> Thanks,
> Srini
>
>>   .../ABI/testing/sysfs-driver-jz4780-efuse  |  16 ++
>
>
>>   .../bindings/nvmem/ingenic,jz4780-efuse.txt|  17 ++
>>   MAINTAINERS|   5 +
>>   arch/mips/boot/dts/ingenic/jz4780.dtsi |  40 ++-
>>   drivers/nvmem/Kconfig  |  10 +
>>   drivers/nvmem/Makefile |   2 +
>>   drivers/nvmem/jz4780-efuse.c   | 305
>> +
>>   7 files changed, 383 insertions(+), 12 deletions(-)
>
> ...

Regards,
PrasannaKumar


Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2018-01-02 Thread Srinivas Kandagatla



On 28/12/17 21:29, Mathieu Malaterre wrote:

From: PrasannaKumar Muralidharan 

This patch brings support for the JZ4780 efuse. Currently it only expose
a read only access to the entire 8K bits efuse memory.

Tested-by: Mathieu Malaterre 
Signed-off-by: PrasannaKumar Muralidharan 
Signed-off-by: Mathieu Malaterre 
---
Please split this patch, as you are mixing code, documentation, dts and 
MAINTAINER changes here.


Without which patch can not be reviewed!!


Thanks,
Srini


  .../ABI/testing/sysfs-driver-jz4780-efuse  |  16 ++



  .../bindings/nvmem/ingenic,jz4780-efuse.txt|  17 ++
  MAINTAINERS|   5 +
  arch/mips/boot/dts/ingenic/jz4780.dtsi |  40 ++-
  drivers/nvmem/Kconfig  |  10 +
  drivers/nvmem/Makefile |   2 +
  drivers/nvmem/jz4780-efuse.c   | 305 +
  7 files changed, 383 insertions(+), 12 deletions(-)

...


Re: [PATCH v2 1/2] nvmem: add driver for JZ4780 efuse

2017-12-29 Thread Marcin Nowakowski

Hi Mathieu,

On 28.12.2017 22:29, Mathieu Malaterre wrote:


--- /dev/null
+++ b/drivers/nvmem/jz4780-efuse.c
@@ -0,0 +1,305 @@
+/*
+ * JZ4780 EFUSE Memory Support driver
+ *
+ * Copyright (c) 2017 PrasannaKumar Muralidharan 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+


Can you use SPDX identifier instead?


+/*
+ * Currently supports JZ4780 efuse which has 8K programmable bit.
+ * Efuse is separated into seven segments as below:
+ *
+ * ---
+ * | 64 bit | 128 bit | 128 bit | 3520 bit | 8 bit | 2296 bit | 2048 bit |
+ * ---
+ *
+ * The rom itself is accessed using a 9 bit address line and an 8 word wide bus
+ * which reads/writes based on strobes. The strobe is configured in the config
+ * register and is based on number of cycles of the bus clock.
+ *
+ * Driver supports read only as the writes are done in the Factory.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define JZ_EFUCTRL (0x0)   /* Control Register */
+#define JZ_EFUCFG  (0x4)   /* Configure Register*/
+#define JZ_EFUSTATE(0x8)   /* Status Register */
+#define JZ_EFUDATA(n)  (0xC + (n)*4)
+
+#define JZ_EFUSE_START_ADDR0x200
+#define JZ_EFUSE_SEG1_OFF  0x00/* 64 bit Random Number */
+#define JZ_EFUSE_SEG2_OFF  0x08/* 128 bit Ingenic Chip ID */
+#define JZ_EFUSE_SEG3_OFF  0x18/* 128 bit Customer ID */
+#define JZ_EFUSE_SEG4_OFF  0x28/* 3520 bit Reserved */
+#define JZ_EFUSE_SEG5_OFF  0x1E0   /* 8 bit Protect Segment */
+#define JZ_EFUSE_SEG6_OFF  0x1E1   /* 2296 bit HDMI Key */
+#define JZ_EFUSE_SEG7_OFF  0x300   /* 2048 bit Security boot key */
+#define JZ_EFUSE_END_ADDR  0x5FF
+
+#define JZ_EFUSE_EFUCTRL_CSBIT(30)
+#define JZ_EFUSE_EFUCTRL_ADDR_MASK 0x1FF
+#define JZ_EFUSE_EFUCTRL_ADDR_SHIFT21
+#define JZ_EFUSE_EFUCTRL_LEN_MASK  0x1F
+#define JZ_EFUSE_EFUCTRL_LEN_SHIFT 16
+#define JZ_EFUSE_EFUCTRL_PG_EN BIT(15)
+#define JZ_EFUSE_EFUCTRL_WR_EN BIT(1)
+#define JZ_EFUSE_EFUCTRL_RD_EN BIT(0)
+
+#define JZ_EFUSE_EFUCFG_INT_EN BIT(31)
+#define JZ_EFUSE_EFUCFG_RD_ADJ_MASK0xF
+#define JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT   20
+#define JZ_EFUSE_EFUCFG_RD_STR_MASK0xF
+#define JZ_EFUSE_EFUCFG_RD_STR_SHIFT   16
+#define JZ_EFUSE_EFUCFG_WR_ADJ_MASK0xF
+#define JZ_EFUSE_EFUCFG_WR_ADJ_SHIFT   12
+#define JZ_EFUSE_EFUCFG_WR_STR_MASK0xFFF
+#define JZ_EFUSE_EFUCFG_WR_STR_SHIFT   0
+
+#define JZ_EFUSE_EFUSTATE_WR_DONE  BIT(1)
+#define JZ_EFUSE_EFUSTATE_RD_DONE  BIT(0)
+
+#define JZ_EFUSE_WORD_SIZE 16
+#define JZ_EFUSE_STRIDE8
+
+struct jz4780_efuse {
+   struct device *dev;
+   void __iomem *iomem;
+   struct clk *clk;
+   unsigned int rd_adj;
+   unsigned int rd_strobe;
+};
+
+/* We read 32 byte chunks to avoid complexity in the driver. */
+static int jz4780_efuse_read_32bytes(struct jz4780_efuse *efuse, char *buf,
+   unsigned int addr)
+{
+   unsigned int tmp = 0;
+   int i = 0;
+   int timeout = 1000;
+   int size = 32;
+
+   /* 1. Set config register */
+   tmp = readl(efuse->iomem + JZ_EFUCFG);
+   tmp &= ~((JZ_EFUSE_EFUCFG_RD_ADJ_MASK << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+   | (JZ_EFUSE_EFUCFG_RD_STR_MASK << JZ_EFUSE_EFUCFG_RD_STR_SHIFT));
+   tmp |= (efuse->rd_adj << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+   | (efuse->rd_strobe << JZ_EFUSE_EFUCFG_RD_STR_SHIFT);
+   writel(tmp, efuse->iomem + JZ_EFUCFG);
+
+   /*
+* 2. Set control register to indicate what to read data address,
+* read data numbers and read enable.
+*/
+   tmp = readl(efuse->iomem + JZ_EFUCTRL);
+   tmp &= ~(JZ_EFUSE_EFUCFG_RD_STR_SHIFT
+   | (JZ_EFUSE_EFUCTRL_ADDR_MASK << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+   | JZ_EFUSE_EFUCTRL_PG_EN | JZ_EFUSE_EFUCTRL_WR_EN
+   | JZ_EFUSE_EFUCTRL_WR_EN);
+
+   /* Need to select CS bit if address accesses upper 4Kbits memory */
+   if (addr >= (JZ_EFUSE_START_ADDR + 512))
+   tmp |= JZ_EFUSE_EFUCTRL_CS;
+
+   tmp |= (addr << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+   | ((size - 1) << JZ_EFUSE_EFUCTRL_LEN_SHIFT)
+   | JZ_EFUSE_EFUCTRL_RD_EN;
+   writel(tmp, efuse->iomem + JZ_EFUCTRL);
+
+   /*
+* 3. Wait status register RD_DONE set to 1 or EFUSE interrupted,
+* software can read EFUSE data buffer 0 - 8 registers.
+*/
+   do {
+   tmp = readl(efuse->iomem