Re: [PATCH 06/14] misc: Add support for nvmem cells

2022-03-14 Thread Sean Anderson



On 3/11/22 9:25 PM, Simon Glass wrote:
> Hi Sean,
> 
> On Thu, 3 Mar 2022 at 10:45, Sean Anderson  wrote:
>>
>> Hi Simon,
>>
>> On 3/1/22 9:58 AM, Simon Glass wrote:
>> > Hi Sean,
>> >
>> > On Mon, 28 Feb 2022 at 09:43, Sean Anderson  wrote:
>> >>
>> >>
>> >>
>> >> On 2/26/22 1:36 PM, Simon Glass wrote:
>> >> > Hi Sean,
>> >> >
>> >> > On Mon, 7 Feb 2022 at 16:42, Sean Anderson  
>> >> > wrote:
>> >> >>
>> >> >> This adds support for "nvmem cells" as seen in Linux. The nvmem device
>> >> >> class in Linux is used for various assorted ROMs and EEPROMs. In this
>> >> >> sense, it is similar to UCLASS_MISC, but also includes
>> >> >> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. While nvmem devices can
>> >> >> be accessed directly, they are most often used by reading/writing
>> >> >> contiguous values called "cells". Cells typically hold information like
>> >> >> calibration, versions, or configuration (such as mac addresses).
>> >> >>
>> >> >> nvmem devices can specify "cells" in their device tree:
>> >> >>
>> >> >> qfprom: eeprom@70 {
>> >> >> #address-cells = <1>;
>> >> >> #size-cells = <1>;
>> >> >> reg = <0x0070 0x10>;
>> >> >>
>> >> >> /* ... */
>> >> >>
>> >> >> tsens_calibration: calib@404 {
>> >> >> reg = <0x404 0x10>;
>> >> >> };
>> >> >> };
>> >> >>
>> >> >> which can then be referenced like:
>> >> >>
>> >> >> tsens {
>> >> >> /* ... */
>> >> >> nvmem-cells = <_calibration>;
>> >> >> nvmem-cell-names = "calibration";
>> >> >> };
>> >> >>
>> >> >> The tsens driver could then read the calibration value like:
>> >> >>
>> >> >> struct nvmem_cell cal_cell;
>> >> >> u8 cal[16];
>> >> >> nvmem_cell_get_by_name(dev, "calibration", _cell);
>> >> >> nvmem_cell_read(_cell, cal, sizeof(cal));
>> >> >>
>> >> >> Because nvmem devices are not all of the same uclass, supported 
>> >> >> uclasses
>> >> >> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be
>> >> >> enabled without depending on specific uclasses. At the moment,
>> >> >> nvmem_interface is very bare-bones, and assumes that no initialization
>> >> >> is necessary. However, this could be amended in the future.
>> >> >>
>> >> >> Although I2C_EEPROM and MISC are quite similar (and could likely be
>> >> >> unified), they present different read/write function signatures. To
>> >> >> abstract over this, NVMEM uses the same read/write signature as Linux.
>> >> >> In particular, short read/writes are not allowed, which is allowed by
>> >> >> MISC.
>> >> >>
>> >> >> The functionality implemented by nvmem cells is very similar to that
>> >> >> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does
>> >> >> not seem to have made its way into Linux or into any device tree other
>> >> >> than sandbox. It is possible that with the introduction of this API it
>> >> >> would be possible to remove it.
>> >> >>
>> >> >> Signed-off-by: Sean Anderson 
>> >> >> ---
>> >> >>
>> >> >>  MAINTAINERS   |   7 ++
>> >> >>  doc/api/index.rst |   1 +
>> >> >>  doc/api/nvmem.rst |   7 ++
>> >> >>  drivers/misc/Kconfig  |  16 
>> >> >>  drivers/misc/Makefile |   1 +
>> >> >>  drivers/misc/nvmem.c  | 109 +
>> >> >>  include/nvmem.h   | 185 ++
>> >> >>  7 files changed, 326 insertions(+)
>> >> >>  create mode 100644 doc/api/nvmem.rst
>> >> >>  create mode 100644 drivers/misc/nvmem.c
>> >> >>  create mode 100644 include/nvmem.h
>> >> >
>> >> > Here I think it would be better to add a new uclass so that drivers
>> >> > which support it can add a child device in that uclass. This is done
>> >> > in lots of places in U-Boot.
>> >>
>> >> I'm not sure exactly what you have in mind. The issue is that there are at
>> >> least 6 uclasses which I would like to support:
>> >>
>> >> - UCLASS_MISC
>> >> - UCLASS_I2C_EEPROM
>> >> - UCLASS_RTC
>> >> - UCLASS_MTD
>> >> - UCLASS_FUSE (doesn't exist yet, but probably should)
>> >> - Possibly UCLASS_PMIC
>> >>
>> >> Most of these uclasses have existing interfaces which expose an 
>> >> NVMEM-like API,
>> >> in addition to other uclass-specific functionality. Instead of having an
>> >> additional API which drivers must implement, I would like to leverage 
>> >> these
>> >> existing APIs to make adding NVMEM support as painless as possible. NVMEM 
>> >> is
>> >> more of a "meta-uclass" which allows us to leverage existing read/write
>> >> functions in uclasses. If any additional devices are to be created, they 
>> >> need
>> >> to be created by the nvmem subsystem, or by the supported uclasses, 
>> >> rather than
>> >> in drivers.
>> >
>> > I may be missing something as I have not looked in detail at your API 
>> > changes.
>> >
>> > But the way to have a consistent API is to use a uclass. 

Re: [PATCH 06/14] misc: Add support for nvmem cells

2022-03-11 Thread Simon Glass
Hi Sean,

On Thu, 3 Mar 2022 at 10:45, Sean Anderson  wrote:
>
> Hi Simon,
>
> On 3/1/22 9:58 AM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Mon, 28 Feb 2022 at 09:43, Sean Anderson  wrote:
> >>
> >>
> >>
> >> On 2/26/22 1:36 PM, Simon Glass wrote:
> >> > Hi Sean,
> >> >
> >> > On Mon, 7 Feb 2022 at 16:42, Sean Anderson  
> >> > wrote:
> >> >>
> >> >> This adds support for "nvmem cells" as seen in Linux. The nvmem device
> >> >> class in Linux is used for various assorted ROMs and EEPROMs. In this
> >> >> sense, it is similar to UCLASS_MISC, but also includes
> >> >> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. While nvmem devices can
> >> >> be accessed directly, they are most often used by reading/writing
> >> >> contiguous values called "cells". Cells typically hold information like
> >> >> calibration, versions, or configuration (such as mac addresses).
> >> >>
> >> >> nvmem devices can specify "cells" in their device tree:
> >> >>
> >> >> qfprom: eeprom@70 {
> >> >> #address-cells = <1>;
> >> >> #size-cells = <1>;
> >> >> reg = <0x0070 0x10>;
> >> >>
> >> >> /* ... */
> >> >>
> >> >> tsens_calibration: calib@404 {
> >> >> reg = <0x404 0x10>;
> >> >> };
> >> >> };
> >> >>
> >> >> which can then be referenced like:
> >> >>
> >> >> tsens {
> >> >> /* ... */
> >> >> nvmem-cells = <_calibration>;
> >> >> nvmem-cell-names = "calibration";
> >> >> };
> >> >>
> >> >> The tsens driver could then read the calibration value like:
> >> >>
> >> >> struct nvmem_cell cal_cell;
> >> >> u8 cal[16];
> >> >> nvmem_cell_get_by_name(dev, "calibration", _cell);
> >> >> nvmem_cell_read(_cell, cal, sizeof(cal));
> >> >>
> >> >> Because nvmem devices are not all of the same uclass, supported uclasses
> >> >> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be
> >> >> enabled without depending on specific uclasses. At the moment,
> >> >> nvmem_interface is very bare-bones, and assumes that no initialization
> >> >> is necessary. However, this could be amended in the future.
> >> >>
> >> >> Although I2C_EEPROM and MISC are quite similar (and could likely be
> >> >> unified), they present different read/write function signatures. To
> >> >> abstract over this, NVMEM uses the same read/write signature as Linux.
> >> >> In particular, short read/writes are not allowed, which is allowed by
> >> >> MISC.
> >> >>
> >> >> The functionality implemented by nvmem cells is very similar to that
> >> >> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does
> >> >> not seem to have made its way into Linux or into any device tree other
> >> >> than sandbox. It is possible that with the introduction of this API it
> >> >> would be possible to remove it.
> >> >>
> >> >> Signed-off-by: Sean Anderson 
> >> >> ---
> >> >>
> >> >>  MAINTAINERS   |   7 ++
> >> >>  doc/api/index.rst |   1 +
> >> >>  doc/api/nvmem.rst |   7 ++
> >> >>  drivers/misc/Kconfig  |  16 
> >> >>  drivers/misc/Makefile |   1 +
> >> >>  drivers/misc/nvmem.c  | 109 +
> >> >>  include/nvmem.h   | 185 ++
> >> >>  7 files changed, 326 insertions(+)
> >> >>  create mode 100644 doc/api/nvmem.rst
> >> >>  create mode 100644 drivers/misc/nvmem.c
> >> >>  create mode 100644 include/nvmem.h
> >> >
> >> > Here I think it would be better to add a new uclass so that drivers
> >> > which support it can add a child device in that uclass. This is done
> >> > in lots of places in U-Boot.
> >>
> >> I'm not sure exactly what you have in mind. The issue is that there are at
> >> least 6 uclasses which I would like to support:
> >>
> >> - UCLASS_MISC
> >> - UCLASS_I2C_EEPROM
> >> - UCLASS_RTC
> >> - UCLASS_MTD
> >> - UCLASS_FUSE (doesn't exist yet, but probably should)
> >> - Possibly UCLASS_PMIC
> >>
> >> Most of these uclasses have existing interfaces which expose an NVMEM-like 
> >> API,
> >> in addition to other uclass-specific functionality. Instead of having an
> >> additional API which drivers must implement, I would like to leverage these
> >> existing APIs to make adding NVMEM support as painless as possible. NVMEM 
> >> is
> >> more of a "meta-uclass" which allows us to leverage existing read/write
> >> functions in uclasses. If any additional devices are to be created, they 
> >> need
> >> to be created by the nvmem subsystem, or by the supported uclasses, rather 
> >> than
> >> in drivers.
> >
> > I may be missing something as I have not looked in detail at your API 
> > changes.
> >
> > But the way to have a consistent API is to use a uclass. We do this
> > with BLK. When a PMIC have GPIOs, RTC and regulators, we add them as
> > child devices. We also have it with bootstd, where a bootdev is
> > created as a child device 

Re: [PATCH 06/14] misc: Add support for nvmem cells

2022-03-03 Thread Sean Anderson
Hi Simon,

On 3/1/22 9:58 AM, Simon Glass wrote:
> Hi Sean,
> 
> On Mon, 28 Feb 2022 at 09:43, Sean Anderson  wrote:
>>
>>
>>
>> On 2/26/22 1:36 PM, Simon Glass wrote:
>> > Hi Sean,
>> >
>> > On Mon, 7 Feb 2022 at 16:42, Sean Anderson  wrote:
>> >>
>> >> This adds support for "nvmem cells" as seen in Linux. The nvmem device
>> >> class in Linux is used for various assorted ROMs and EEPROMs. In this
>> >> sense, it is similar to UCLASS_MISC, but also includes
>> >> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. While nvmem devices can
>> >> be accessed directly, they are most often used by reading/writing
>> >> contiguous values called "cells". Cells typically hold information like
>> >> calibration, versions, or configuration (such as mac addresses).
>> >>
>> >> nvmem devices can specify "cells" in their device tree:
>> >>
>> >> qfprom: eeprom@70 {
>> >> #address-cells = <1>;
>> >> #size-cells = <1>;
>> >> reg = <0x0070 0x10>;
>> >>
>> >> /* ... */
>> >>
>> >> tsens_calibration: calib@404 {
>> >> reg = <0x404 0x10>;
>> >> };
>> >> };
>> >>
>> >> which can then be referenced like:
>> >>
>> >> tsens {
>> >> /* ... */
>> >> nvmem-cells = <_calibration>;
>> >> nvmem-cell-names = "calibration";
>> >> };
>> >>
>> >> The tsens driver could then read the calibration value like:
>> >>
>> >> struct nvmem_cell cal_cell;
>> >> u8 cal[16];
>> >> nvmem_cell_get_by_name(dev, "calibration", _cell);
>> >> nvmem_cell_read(_cell, cal, sizeof(cal));
>> >>
>> >> Because nvmem devices are not all of the same uclass, supported uclasses
>> >> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be
>> >> enabled without depending on specific uclasses. At the moment,
>> >> nvmem_interface is very bare-bones, and assumes that no initialization
>> >> is necessary. However, this could be amended in the future.
>> >>
>> >> Although I2C_EEPROM and MISC are quite similar (and could likely be
>> >> unified), they present different read/write function signatures. To
>> >> abstract over this, NVMEM uses the same read/write signature as Linux.
>> >> In particular, short read/writes are not allowed, which is allowed by
>> >> MISC.
>> >>
>> >> The functionality implemented by nvmem cells is very similar to that
>> >> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does
>> >> not seem to have made its way into Linux or into any device tree other
>> >> than sandbox. It is possible that with the introduction of this API it
>> >> would be possible to remove it.
>> >>
>> >> Signed-off-by: Sean Anderson 
>> >> ---
>> >>
>> >>  MAINTAINERS   |   7 ++
>> >>  doc/api/index.rst |   1 +
>> >>  doc/api/nvmem.rst |   7 ++
>> >>  drivers/misc/Kconfig  |  16 
>> >>  drivers/misc/Makefile |   1 +
>> >>  drivers/misc/nvmem.c  | 109 +
>> >>  include/nvmem.h   | 185 ++
>> >>  7 files changed, 326 insertions(+)
>> >>  create mode 100644 doc/api/nvmem.rst
>> >>  create mode 100644 drivers/misc/nvmem.c
>> >>  create mode 100644 include/nvmem.h
>> >
>> > Here I think it would be better to add a new uclass so that drivers
>> > which support it can add a child device in that uclass. This is done
>> > in lots of places in U-Boot.
>>
>> I'm not sure exactly what you have in mind. The issue is that there are at
>> least 6 uclasses which I would like to support:
>>
>> - UCLASS_MISC
>> - UCLASS_I2C_EEPROM
>> - UCLASS_RTC
>> - UCLASS_MTD
>> - UCLASS_FUSE (doesn't exist yet, but probably should)
>> - Possibly UCLASS_PMIC
>>
>> Most of these uclasses have existing interfaces which expose an NVMEM-like 
>> API,
>> in addition to other uclass-specific functionality. Instead of having an
>> additional API which drivers must implement, I would like to leverage these
>> existing APIs to make adding NVMEM support as painless as possible. NVMEM is
>> more of a "meta-uclass" which allows us to leverage existing read/write
>> functions in uclasses. If any additional devices are to be created, they need
>> to be created by the nvmem subsystem, or by the supported uclasses, rather 
>> than
>> in drivers.
> 
> I may be missing something as I have not looked in detail at your API changes.
> 
> But the way to have a consistent API is to use a uclass. We do this
> with BLK. When a PMIC have GPIOs, RTC and regulators, we add them as
> child devices. We also have it with bootstd, where a bootdev is
> created as a child device of a storage device. We can put the required
> stuff in a helper function. We can even avoid any new code in the
> drivers by using the pending event system.
> 
> Can you first help me understand what is wrong with using a new uclass?

I suppose it could be done this way.

Effectively, we are "picking" out two 

Re: [PATCH 06/14] misc: Add support for nvmem cells

2022-03-01 Thread Simon Glass
Hi Sean,

On Mon, 28 Feb 2022 at 09:43, Sean Anderson  wrote:
>
>
>
> On 2/26/22 1:36 PM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Mon, 7 Feb 2022 at 16:42, Sean Anderson  wrote:
> >>
> >> This adds support for "nvmem cells" as seen in Linux. The nvmem device
> >> class in Linux is used for various assorted ROMs and EEPROMs. In this
> >> sense, it is similar to UCLASS_MISC, but also includes
> >> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. While nvmem devices can
> >> be accessed directly, they are most often used by reading/writing
> >> contiguous values called "cells". Cells typically hold information like
> >> calibration, versions, or configuration (such as mac addresses).
> >>
> >> nvmem devices can specify "cells" in their device tree:
> >>
> >> qfprom: eeprom@70 {
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >> reg = <0x0070 0x10>;
> >>
> >> /* ... */
> >>
> >> tsens_calibration: calib@404 {
> >> reg = <0x404 0x10>;
> >> };
> >> };
> >>
> >> which can then be referenced like:
> >>
> >> tsens {
> >> /* ... */
> >> nvmem-cells = <_calibration>;
> >> nvmem-cell-names = "calibration";
> >> };
> >>
> >> The tsens driver could then read the calibration value like:
> >>
> >> struct nvmem_cell cal_cell;
> >> u8 cal[16];
> >> nvmem_cell_get_by_name(dev, "calibration", _cell);
> >> nvmem_cell_read(_cell, cal, sizeof(cal));
> >>
> >> Because nvmem devices are not all of the same uclass, supported uclasses
> >> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be
> >> enabled without depending on specific uclasses. At the moment,
> >> nvmem_interface is very bare-bones, and assumes that no initialization
> >> is necessary. However, this could be amended in the future.
> >>
> >> Although I2C_EEPROM and MISC are quite similar (and could likely be
> >> unified), they present different read/write function signatures. To
> >> abstract over this, NVMEM uses the same read/write signature as Linux.
> >> In particular, short read/writes are not allowed, which is allowed by
> >> MISC.
> >>
> >> The functionality implemented by nvmem cells is very similar to that
> >> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does
> >> not seem to have made its way into Linux or into any device tree other
> >> than sandbox. It is possible that with the introduction of this API it
> >> would be possible to remove it.
> >>
> >> Signed-off-by: Sean Anderson 
> >> ---
> >>
> >>  MAINTAINERS   |   7 ++
> >>  doc/api/index.rst |   1 +
> >>  doc/api/nvmem.rst |   7 ++
> >>  drivers/misc/Kconfig  |  16 
> >>  drivers/misc/Makefile |   1 +
> >>  drivers/misc/nvmem.c  | 109 +
> >>  include/nvmem.h   | 185 ++
> >>  7 files changed, 326 insertions(+)
> >>  create mode 100644 doc/api/nvmem.rst
> >>  create mode 100644 drivers/misc/nvmem.c
> >>  create mode 100644 include/nvmem.h
> >
> > Here I think it would be better to add a new uclass so that drivers
> > which support it can add a child device in that uclass. This is done
> > in lots of places in U-Boot.
>
> I'm not sure exactly what you have in mind. The issue is that there are at
> least 6 uclasses which I would like to support:
>
> - UCLASS_MISC
> - UCLASS_I2C_EEPROM
> - UCLASS_RTC
> - UCLASS_MTD
> - UCLASS_FUSE (doesn't exist yet, but probably should)
> - Possibly UCLASS_PMIC
>
> Most of these uclasses have existing interfaces which expose an NVMEM-like 
> API,
> in addition to other uclass-specific functionality. Instead of having an
> additional API which drivers must implement, I would like to leverage these
> existing APIs to make adding NVMEM support as painless as possible. NVMEM is
> more of a "meta-uclass" which allows us to leverage existing read/write
> functions in uclasses. If any additional devices are to be created, they need
> to be created by the nvmem subsystem, or by the supported uclasses, rather 
> than
> in drivers.

I may be missing something as I have not looked in detail at your API changes.

But the way to have a consistent API is to use a uclass. We do this
with BLK. When a PMIC have GPIOs, RTC and regulators, we add them as
child devices. We also have it with bootstd, where a bootdev is
created as a child device of a storage device. We can put the required
stuff in a helper function. We can even avoid any new code in the
drivers by using the pending event system.

Can you first help me understand what is wrong with using a new uclass?

>
> Now, there are some instances where creating a new child device might be the
> best approach. For example, you might have some OTP memory in an unrelated
> device. In that case, creating a child device (of UCLASS_MISC, or UCLASS_FUSE
> if that gets 

Re: [PATCH 06/14] misc: Add support for nvmem cells

2022-02-28 Thread Sean Anderson



On 2/26/22 1:36 PM, Simon Glass wrote:
> Hi Sean,
> 
> On Mon, 7 Feb 2022 at 16:42, Sean Anderson  wrote:
>>
>> This adds support for "nvmem cells" as seen in Linux. The nvmem device
>> class in Linux is used for various assorted ROMs and EEPROMs. In this
>> sense, it is similar to UCLASS_MISC, but also includes
>> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. While nvmem devices can
>> be accessed directly, they are most often used by reading/writing
>> contiguous values called "cells". Cells typically hold information like
>> calibration, versions, or configuration (such as mac addresses).
>>
>> nvmem devices can specify "cells" in their device tree:
>>
>> qfprom: eeprom@70 {
>> #address-cells = <1>;
>> #size-cells = <1>;
>> reg = <0x0070 0x10>;
>>
>> /* ... */
>>
>> tsens_calibration: calib@404 {
>> reg = <0x404 0x10>;
>> };
>> };
>>
>> which can then be referenced like:
>>
>> tsens {
>> /* ... */
>> nvmem-cells = <_calibration>;
>> nvmem-cell-names = "calibration";
>> };
>>
>> The tsens driver could then read the calibration value like:
>>
>> struct nvmem_cell cal_cell;
>> u8 cal[16];
>> nvmem_cell_get_by_name(dev, "calibration", _cell);
>> nvmem_cell_read(_cell, cal, sizeof(cal));
>>
>> Because nvmem devices are not all of the same uclass, supported uclasses
>> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be
>> enabled without depending on specific uclasses. At the moment,
>> nvmem_interface is very bare-bones, and assumes that no initialization
>> is necessary. However, this could be amended in the future.
>>
>> Although I2C_EEPROM and MISC are quite similar (and could likely be
>> unified), they present different read/write function signatures. To
>> abstract over this, NVMEM uses the same read/write signature as Linux.
>> In particular, short read/writes are not allowed, which is allowed by
>> MISC.
>>
>> The functionality implemented by nvmem cells is very similar to that
>> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does
>> not seem to have made its way into Linux or into any device tree other
>> than sandbox. It is possible that with the introduction of this API it
>> would be possible to remove it.
>>
>> Signed-off-by: Sean Anderson 
>> ---
>>
>>  MAINTAINERS   |   7 ++
>>  doc/api/index.rst |   1 +
>>  doc/api/nvmem.rst |   7 ++
>>  drivers/misc/Kconfig  |  16 
>>  drivers/misc/Makefile |   1 +
>>  drivers/misc/nvmem.c  | 109 +
>>  include/nvmem.h   | 185 ++
>>  7 files changed, 326 insertions(+)
>>  create mode 100644 doc/api/nvmem.rst
>>  create mode 100644 drivers/misc/nvmem.c
>>  create mode 100644 include/nvmem.h
> 
> Here I think it would be better to add a new uclass so that drivers
> which support it can add a child device in that uclass. This is done
> in lots of places in U-Boot.

I'm not sure exactly what you have in mind. The issue is that there are at
least 6 uclasses which I would like to support:

- UCLASS_MISC
- UCLASS_I2C_EEPROM
- UCLASS_RTC
- UCLASS_MTD
- UCLASS_FUSE (doesn't exist yet, but probably should)
- Possibly UCLASS_PMIC

Most of these uclasses have existing interfaces which expose an NVMEM-like API,
in addition to other uclass-specific functionality. Instead of having an
additional API which drivers must implement, I would like to leverage these
existing APIs to make adding NVMEM support as painless as possible. NVMEM is
more of a "meta-uclass" which allows us to leverage existing read/write
functions in uclasses. If any additional devices are to be created, they need
to be created by the nvmem subsystem, or by the supported uclasses, rather than
in drivers.

Now, there are some instances where creating a new child device might be the
best approach. For example, you might have some OTP memory in an unrelated
device. In that case, creating a child device (of UCLASS_MISC, or UCLASS_FUSE
if that gets created) is the right course of action.

--Sean


Re: [PATCH 06/14] misc: Add support for nvmem cells

2022-02-26 Thread Simon Glass
Hi Sean,

On Mon, 7 Feb 2022 at 16:42, Sean Anderson  wrote:
>
> This adds support for "nvmem cells" as seen in Linux. The nvmem device
> class in Linux is used for various assorted ROMs and EEPROMs. In this
> sense, it is similar to UCLASS_MISC, but also includes
> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. While nvmem devices can
> be accessed directly, they are most often used by reading/writing
> contiguous values called "cells". Cells typically hold information like
> calibration, versions, or configuration (such as mac addresses).
>
> nvmem devices can specify "cells" in their device tree:
>
> qfprom: eeprom@70 {
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x0070 0x10>;
>
> /* ... */
>
> tsens_calibration: calib@404 {
> reg = <0x404 0x10>;
> };
> };
>
> which can then be referenced like:
>
> tsens {
> /* ... */
> nvmem-cells = <_calibration>;
> nvmem-cell-names = "calibration";
> };
>
> The tsens driver could then read the calibration value like:
>
> struct nvmem_cell cal_cell;
> u8 cal[16];
> nvmem_cell_get_by_name(dev, "calibration", _cell);
> nvmem_cell_read(_cell, cal, sizeof(cal));
>
> Because nvmem devices are not all of the same uclass, supported uclasses
> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be
> enabled without depending on specific uclasses. At the moment,
> nvmem_interface is very bare-bones, and assumes that no initialization
> is necessary. However, this could be amended in the future.
>
> Although I2C_EEPROM and MISC are quite similar (and could likely be
> unified), they present different read/write function signatures. To
> abstract over this, NVMEM uses the same read/write signature as Linux.
> In particular, short read/writes are not allowed, which is allowed by
> MISC.
>
> The functionality implemented by nvmem cells is very similar to that
> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does
> not seem to have made its way into Linux or into any device tree other
> than sandbox. It is possible that with the introduction of this API it
> would be possible to remove it.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  MAINTAINERS   |   7 ++
>  doc/api/index.rst |   1 +
>  doc/api/nvmem.rst |   7 ++
>  drivers/misc/Kconfig  |  16 
>  drivers/misc/Makefile |   1 +
>  drivers/misc/nvmem.c  | 109 +
>  include/nvmem.h   | 185 ++
>  7 files changed, 326 insertions(+)
>  create mode 100644 doc/api/nvmem.rst
>  create mode 100644 drivers/misc/nvmem.c
>  create mode 100644 include/nvmem.h

Here I think it would be better to add a new uclass so that drivers
which support it can add a child device in that uclass. This is done
in lots of places in U-Boot.

Regards,
Simon