2013-07-05 20:11:22

by Maxime Ripard

[permalink] [raw]
Subject: MTD EEPROM support and driver integration

Hi everyone,

In the last weeks, we've drivers coming up both about mostly some very
simple drivers that expose to the userspace a few bytes of memory-mapped
IO. Both will probably live under drivers/misc/eeprom, where there's
support already for other kind of what could be assimilated as eeproms
(AT24, AT25, etc.).

Now, besides the code duplication every driver has to make to register
the sysfs attributes, it wouldn't cause much of a problem.

Except that these EEPROMs could store values useful for other drivers in
Linux. For example:
- imx28 OCOTP (the latest of the two EEPROM drivers recently submitted)
use it most of the time to store its MAC addresses
- Allwinner SoCs SID (the other latest driver submitted) use it
sometime to store a MAC address, and also the SoC ID, which could be
useful to implement a SoC bus driver.
- Some Allwinner boards (and presumably others) use an AT24 to store
the MAC address in it.

For now, everyone comes up with a different solution:
- imx28 has a hook in mach-mxs to patch the device tree at runtime and
add the values retrieved from the OCOTP to it.
- AT24 seem to have some function exported (at24_macc_(read|write)) so
that other part of the kernel can use them to retrieve values from
such an EEPROM.
- Allwinner SoCs have, well, basically nothing for now, which is why I
send this email.

The current way of working has several flaws imho:
- The code is heavily duplicated: the sysfs registering is common to
every driver, and at the end of the day, every driver should only
give a read/write callback, and that's it.
- The "consumer" drivers should not have to worry at all about the
EEPROM type it should retrieve the needed value from, let alone
dealing with the number of instances of such an EEPROM.

To solve this issues, I think I have some solution. Would merging this
drivers into MTD make some sense? It seems like there is already some
EEPROM drivers into drivers/mtd (such as an AT25 one, which also have a
drivers under drivers/misc/eeprom), so I guess it does, but I'd like to
have your opinion on this.

If so, would some kind of MTD in-kernel API to retrieve values from MTD
devices would be acceptable to you? I mostly have DT in mind, so I'm
thinking of having DT bindings to that API such as

mac-storage = <&phandle 0xoffset 0xsize>

to describe which device to get a value from, and where in that device.

That would allow consumer drivers to only have to call a function like
of_mtd_get_value and let the MTD subsystem do the hard work.

What's your feeling on this?

Thanks,
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (2.67 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-05 21:03:44

by Arnd Bergmann

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Friday 05 July 2013, Maxime Ripard wrote:
> Hi everyone,
>
> In the last weeks, we've drivers coming up both about mostly some very
> simple drivers that expose to the userspace a few bytes of memory-mapped
> IO. Both will probably live under drivers/misc/eeprom, where there's
> support already for other kind of what could be assimilated as eeproms
> (AT24, AT25, etc.).
>
> Now, besides the code duplication every driver has to make to register
> the sysfs attributes, it wouldn't cause much of a problem.
>
> Except that these EEPROMs could store values useful for other drivers in
> Linux. For example:
> - imx28 OCOTP (the latest of the two EEPROM drivers recently submitted)
> use it most of the time to store its MAC addresses
> - Allwinner SoCs SID (the other latest driver submitted) use it
> sometime to store a MAC address, and also the SoC ID, which could be
> useful to implement a SoC bus driver.
> - Some Allwinner boards (and presumably others) use an AT24 to store
> the MAC address in it.
>
> For now, everyone comes up with a different solution:
> - imx28 has a hook in mach-mxs to patch the device tree at runtime and
> add the values retrieved from the OCOTP to it.
> - AT24 seem to have some function exported (at24_macc_(read|write)) so
> that other part of the kernel can use them to retrieve values from
> such an EEPROM.
> - Allwinner SoCs have, well, basically nothing for now, which is why I
> send this email.
>
> The current way of working has several flaws imho:
> - The code is heavily duplicated: the sysfs registering is common to
> every driver, and at the end of the day, every driver should only
> give a read/write callback, and that's it.
> - The "consumer" drivers should not have to worry at all about the
> EEPROM type it should retrieve the needed value from, let alone
> dealing with the number of instances of such an EEPROM.
>
> To solve this issues, I think I have some solution. Would merging this
> drivers into MTD make some sense? It seems like there is already some
> EEPROM drivers into drivers/mtd (such as an AT25 one, which also have a
> drivers under drivers/misc/eeprom), so I guess it does, but I'd like to
> have your opinion on this.

I always felt that we should eventually move the eeprom drivers out of
drivers/misc into their own subsystem. Moving them under drivers/mtd
also seems reasonable.

Having a common API sounds like a good idea, and we should probably
spend some time comparing the options.

> If so, would some kind of MTD in-kernel API to retrieve values from MTD
> devices would be acceptable to you? I mostly have DT in mind, so I'm
> thinking of having DT bindings to that API such as
>
> mac-storage = <&phandle 0xoffset 0xsize>
>
> to describe which device to get a value from, and where in that device.
>
> That would allow consumer drivers to only have to call a function like
> of_mtd_get_value and let the MTD subsystem do the hard work.
>
> What's your feeling on this?

My first thought is that it should be more generic than that and not
have the mac address hardcoded as the purpose. We could possibly use
regmap as the in-kernel interface, and come up with a more generic
way of referring to registers in another device node.

Arnd

2013-07-05 22:23:46

by Maxime Ripard

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

Hi Arnd,

On Fri, Jul 05, 2013 at 11:02:40PM +0200, Arnd Bergmann wrote:
> On Friday 05 July 2013, Maxime Ripard wrote:
> > Hi everyone,
> >
> > In the last weeks, we've drivers coming up both about mostly some very
> > simple drivers that expose to the userspace a few bytes of memory-mapped
> > IO. Both will probably live under drivers/misc/eeprom, where there's
> > support already for other kind of what could be assimilated as eeproms
> > (AT24, AT25, etc.).
> >
> > Now, besides the code duplication every driver has to make to register
> > the sysfs attributes, it wouldn't cause much of a problem.
> >
> > Except that these EEPROMs could store values useful for other drivers in
> > Linux. For example:
> > - imx28 OCOTP (the latest of the two EEPROM drivers recently submitted)
> > use it most of the time to store its MAC addresses
> > - Allwinner SoCs SID (the other latest driver submitted) use it
> > sometime to store a MAC address, and also the SoC ID, which could be
> > useful to implement a SoC bus driver.
> > - Some Allwinner boards (and presumably others) use an AT24 to store
> > the MAC address in it.
> >
> > For now, everyone comes up with a different solution:
> > - imx28 has a hook in mach-mxs to patch the device tree at runtime and
> > add the values retrieved from the OCOTP to it.
> > - AT24 seem to have some function exported (at24_macc_(read|write)) so
> > that other part of the kernel can use them to retrieve values from
> > such an EEPROM.
> > - Allwinner SoCs have, well, basically nothing for now, which is why I
> > send this email.
> >
> > The current way of working has several flaws imho:
> > - The code is heavily duplicated: the sysfs registering is common to
> > every driver, and at the end of the day, every driver should only
> > give a read/write callback, and that's it.
> > - The "consumer" drivers should not have to worry at all about the
> > EEPROM type it should retrieve the needed value from, let alone
> > dealing with the number of instances of such an EEPROM.
> >
> > To solve this issues, I think I have some solution. Would merging this
> > drivers into MTD make some sense? It seems like there is already some
> > EEPROM drivers into drivers/mtd (such as an AT25 one, which also have a
> > drivers under drivers/misc/eeprom), so I guess it does, but I'd like to
> > have your opinion on this.
>
> I always felt that we should eventually move the eeprom drivers out of
> drivers/misc into their own subsystem. Moving them under drivers/mtd
> also seems reasonable.
>
> Having a common API sounds like a good idea, and we should probably
> spend some time comparing the options.

Great :)

> > If so, would some kind of MTD in-kernel API to retrieve values from MTD
> > devices would be acceptable to you? I mostly have DT in mind, so I'm
> > thinking of having DT bindings to that API such as
> >
> > mac-storage = <&phandle 0xoffset 0xsize>
> >
> > to describe which device to get a value from, and where in that device.
> >
> > That would allow consumer drivers to only have to call a function like
> > of_mtd_get_value and let the MTD subsystem do the hard work.
> >
> > What's your feeling on this?
>
> My first thought is that it should be more generic than that and not
> have the mac address hardcoded as the purpose. We could possibly use
> regmap as the in-kernel interface, and come up with a more generic
> way of referring to registers in another device node.

Hmm, I maybe wasn't as clear as I wanted. Here mac-storage was just an
example. It should indeed be completely generic, and a device could have
several "storage source" defined, each driver knowing what property it
would need, pretty much like what's done currently for the regulators
for example.

We will have such a use case anyway for the Allwinner stuff, since the
fuses can be used for several thing, including storing the SoC ID,
serial numbers, and so on.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (4.00 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-05 22:34:12

by Arnd Bergmann

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Saturday 06 July 2013, Maxime Ripard wrote:
> > My first thought is that it should be more generic than that and not
> > have the mac address hardcoded as the purpose. We could possibly use
> > regmap as the in-kernel interface, and come up with a more generic
> > way of referring to registers in another device node.
>
> Hmm, I maybe wasn't as clear as I wanted. Here mac-storage was just an
> example. It should indeed be completely generic, and a device could have
> several "storage source" defined, each driver knowing what property it
> would need, pretty much like what's done currently for the regulators
> for example.
>
> We will have such a use case anyway for the Allwinner stuff, since the
> fuses can be used for several thing, including storing the SoC ID,
> serial numbers, and so on.

Ah, I see. In general, we have two ways of expressing the same thing
here:

a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed property names

regmap = <&at25 0xstart 0xlen>;
regmap-names = "mac-address";

b) like gpio, regulator: variable property names

mac-storage = <&at25 0xstart 0xlen>;

It's unfortunate that we already have examples of both. They are largely
equivalent, but the tendency is towards the first.

Arnd

2013-07-06 08:28:26

by Maxime Ripard

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Sat, Jul 06, 2013 at 12:33:13AM +0200, Arnd Bergmann wrote:
> On Saturday 06 July 2013, Maxime Ripard wrote:
> > > My first thought is that it should be more generic than that and not
> > > have the mac address hardcoded as the purpose. We could possibly use
> > > regmap as the in-kernel interface, and come up with a more generic
> > > way of referring to registers in another device node.
> >
> > Hmm, I maybe wasn't as clear as I wanted. Here mac-storage was just an
> > example. It should indeed be completely generic, and a device could have
> > several "storage source" defined, each driver knowing what property it
> > would need, pretty much like what's done currently for the regulators
> > for example.
> >
> > We will have such a use case anyway for the Allwinner stuff, since the
> > fuses can be used for several thing, including storing the SoC ID,
> > serial numbers, and so on.
>
> Ah, I see. In general, we have two ways of expressing the same thing
> here:
>
> a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed property names
>
> regmap = <&at25 0xstart 0xlen>;
> regmap-names = "mac-address";
>
> b) like gpio, regulator: variable property names
>
> mac-storage = <&at25 0xstart 0xlen>;
>
> It's unfortunate that we already have examples of both. They are largely
> equivalent, but the tendency is towards the first.

I don't have a strong feeling for one against another, so whatever works
best. Both solutions will be a huge improvement anyway :)

Just out of curiosity, is there any advantages besides having a fixed
property name to the first solution?

Thanks,
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.69 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-06 09:16:46

by Arnd Bergmann

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Saturday 06 July 2013 10:28:04 Maxime Ripard wrote:
> > a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed property names
> >
> > regmap = <&at25 0xstart 0xlen>;
> > regmap-names = "mac-address";
> >
> > b) like gpio, regulator: variable property names
> >
> > mac-storage = <&at25 0xstart 0xlen>;
> >
> > It's unfortunate that we already have examples of both. They are largely
> > equivalent, but the tendency is towards the first.
>
> I don't have a strong feeling for one against another, so whatever works
> best. Both solutions will be a huge improvement anyway
>
> Just out of curiosity, is there any advantages besides having a fixed
> property name to the first solution?

I think it's mostly for consistency: trying to get most subsystems to
do it the same way to make it easier for people to write dts files.

A lesser point is that it simplifies the driver code if you don't
have to pass a name.


Arnd

2013-07-06 12:01:15

by Maxime Ripard

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

Hi,

I'm not exactly sure on what happened to the previous mail that has been
sent empty, but anyway:

On Sat, Jul 06, 2013 at 11:18:00AM +0200, Arnd Bergmann wrote:
> On Saturday 06 July 2013 10:28:04 Maxime Ripard wrote:
> > > a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed property names
> > >
> > > regmap = <&at25 0xstart 0xlen>;
> > > regmap-names = "mac-address";
> > >
> > > b) like gpio, regulator: variable property names
> > >
> > > mac-storage = <&at25 0xstart 0xlen>;
> > >
> > > It's unfortunate that we already have examples of both. They are largely
> > > equivalent, but the tendency is towards the first.
> >
> > I don't have a strong feeling for one against another, so whatever works
> > best. Both solutions will be a huge improvement anyway
> >
> > Just out of curiosity, is there any advantages besides having a fixed
> > property name to the first solution?
>
> I think it's mostly for consistency: trying to get most subsystems to
> do it the same way to make it easier for people to write dts files.
>
> A lesser point is that it simplifies the driver code if you don't
> have to pass a name.

So that leave us with mainly one path to achieve this goal:
- Add a regmap-mtd backend
- Add DT parsing code for regmap
- Move the EEPROM drivers from misc to mtd

What other option would we have?

I also thought about writing an EEPROM framework of its own, but the
line is really thin between a large EEPROM and say a small SPI
dataflash, which would make it pretty hard to choose between such a
framework and MTD.

Thanks,
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.67 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-06 19:05:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Saturday 06 July 2013 14:01:12 Maxime Ripard wrote:
> I'm not exactly sure on what happened to the previous mail that has been
> sent empty, but anyway:
>
> On Sat, Jul 06, 2013 at 11:18:00AM +0200, Arnd Bergmann wrote:
> > On Saturday 06 July 2013 10:28:04 Maxime Ripard wrote:
> > > > a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed property names
> > > >
> > > > regmap = <&at25 0xstart 0xlen>;
> > > > regmap-names = "mac-address";
> > > >
> > > > b) like gpio, regulator: variable property names
> > > >
> > > > mac-storage = <&at25 0xstart 0xlen>;
> > > >
> > > > It's unfortunate that we already have examples of both. They are largely
> > > > equivalent, but the tendency is towards the first.
> > >
> > > I don't have a strong feeling for one against another, so whatever works
> > > best. Both solutions will be a huge improvement anyway
> > >
> > > Just out of curiosity, is there any advantages besides having a fixed
> > > property name to the first solution?
> >
> > I think it's mostly for consistency: trying to get most subsystems to
> > do it the same way to make it easier for people to write dts files.
> >
> > A lesser point is that it simplifies the driver code if you don't
> > have to pass a name.
>
> So that leave us with mainly one path to achieve this goal:
> - Add a regmap-mtd backend
> - Add DT parsing code for regmap
> - Move the EEPROM drivers from misc to mtd

Yes, I think that would be good. For the last step, we definitely need
buy-in from Wolfgand and Jean, as they are maintaining the current eeprom
drivers.

> What other option would we have?
>
> I also thought about writing an EEPROM framework of its own, but the
> line is really thin between a large EEPROM and say a small SPI
> dataflash, which would make it pretty hard to choose between such a
> framework and MTD.

Isn't flash by definition block based, while EEPROM can be written
in byte or word units? I think that is a significant difference, although
it doesn't necessarily mean that we can't use MTD for both.

We also have a bunch of OTP drivers spread around the kernel, it probably
makes sense to consolidate them at the same time, at least on the DT binding
side if not the device drivers.

Arnd

2013-07-06 19:56:08

by Jean Delvare

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Sat, 06 Jul 2013 21:06:49 +0200, Arnd Bergmann wrote:
> On Saturday 06 July 2013 14:01:12 Maxime Ripard wrote:
> > So that leave us with mainly one path to achieve this goal:
> > - Add a regmap-mtd backend
> > - Add DT parsing code for regmap
> > - Move the EEPROM drivers from misc to mtd
>
> Yes, I think that would be good. For the last step, we definitely need
> buy-in from Wolfgand and Jean, as they are maintaining the current eeprom
> drivers.

I am maintaining the legacy eeprom driver, which nobody should be using
in conjunction with DT. So you don't need my approval for anything.

--
Jean Delvare

2013-07-07 07:15:08

by Maxime Ripard

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote:
> On Saturday 06 July 2013 14:01:12 Maxime Ripard wrote:
> > What other option would we have?
> >
> > I also thought about writing an EEPROM framework of its own, but the
> > line is really thin between a large EEPROM and say a small SPI
> > dataflash, which would make it pretty hard to choose between such a
> > framework and MTD.
>
> Isn't flash by definition block based, while EEPROM can be written
> in byte or word units? I think that is a significant difference, although
> it doesn't necessarily mean that we can't use MTD for both.

Ah, right.

> We also have a bunch of OTP drivers spread around the kernel, it probably
> makes sense to consolidate them at the same time, at least on the DT binding
> side if not the device drivers.

From a quick grep, the only one I've seen so far are:
- imx6q, that has a hook at machine start to poke into its OCOTP to
retrieve some frequency scaling parameters it seems. I'm not sure
how the current solution could improve the situation for this
use-case, but the DT bindings of the OCOTP is just a DT node, with
no clients, so we have nothing to worry about here.
- imx28, that has a hook at machine start to look up the MAC address
values and patch the ethernet controller nodes to add the right
local-mac-address property. This one could benefit from the new
bindings, but we already mentionned it, and I intended to develop
with an imx28 board anyway.
- picoxcell-pc3x3 DTSI has a node for a OTP device, but they don't
seem to be doing anything with it, nor do they seem to have a driver
for it. So I guess we don't care about migrating for this one
either.

Did you have other cases in mind?

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.83 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-08 08:58:12

by Mark Brown

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote:
> On Saturday 06 July 2013 14:01:12 Maxime Ripard wrote:

> > > > > a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed property names

> > > > > regmap = <&at25 0xstart 0xlen>;
> > > > > regmap-names = "mac-address";

> > > > > b) like gpio, regulator: variable property names

> > > > > mac-storage = <&at25 0xstart 0xlen>;

> > > > > It's unfortunate that we already have examples of both. They are largely
> > > > > equivalent, but the tendency is towards the first.

> > > > I don't have a strong feeling for one against another, so whatever works
> > > > best. Both solutions will be a huge improvement anyway

> > > > Just out of curiosity, is there any advantages besides having a fixed
> > > > property name to the first solution?

> > > I think it's mostly for consistency: trying to get most subsystems to
> > > do it the same way to make it easier for people to write dts files.

> > > A lesser point is that it simplifies the driver code if you don't
> > > have to pass a name.

On the other hand something with human readable names is much more
legible if humans ever have to read or write the DT bindings. This
mostly applies when there are many instances of the property (for
example, many devices have lots of power supplies) or when some
instances of the property are optional (for example, many devices can
use GPIOs for many different functions but usually not all of them are
connected and there's no particular order in which they might get
connected).

> > So that leave us with mainly one path to achieve this goal:
> > - Add a regmap-mtd backend
> > - Add DT parsing code for regmap
> > - Move the EEPROM drivers from misc to mtd

> Yes, I think that would be good. For the last step, we definitely need
> buy-in from Wolfgand and Jean, as they are maintaining the current eeprom
> drivers.

I'd really like to see more discussion of this "DT parsing code for
regmap" idea... I've missed almost all the context here.

> We also have a bunch of OTP drivers spread around the kernel, it probably
> makes sense to consolidate them at the same time, at least on the DT binding
> side if not the device drivers.

I'm not sure how viable this is, the OTP interfaces aren't that
consistent and are frequently embedded in random PMICs or whatever.


Attachments:
(No filename) (2.31 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-08 08:58:35

by Mark Brown

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Sun, Jul 07, 2013 at 09:15:01AM +0200, Maxime Ripard wrote:
> On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote:

> > We also have a bunch of OTP drivers spread around the kernel, it probably
> > makes sense to consolidate them at the same time, at least on the DT binding
> > side if not the device drivers.

> From a quick grep, the only one I've seen so far are:
> - imx6q, that has a hook at machine start to poke into its OCOTP to
> retrieve some frequency scaling parameters it seems. I'm not sure
> how the current solution could improve the situation for this
> use-case, but the DT bindings of the OCOTP is just a DT node, with
> no clients, so we have nothing to worry about here.
> - imx28, that has a hook at machine start to look up the MAC address
> values and patch the ethernet controller nodes to add the right
> local-mac-address property. This one could benefit from the new
> bindings, but we already mentionned it, and I intended to develop
> with an imx28 board anyway.
> - picoxcell-pc3x3 DTSI has a node for a OTP device, but they don't
> seem to be doing anything with it, nor do they seem to have a driver
> for it. So I guess we don't care about migrating for this one
> either.

> Did you have other cases in mind?

We have some OTP support in the ab8500 and wm831x MFD drivers too but
they just expose the data.


Attachments:
(No filename) (1.37 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-08 20:25:45

by Maxime Ripard

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Mon, Jul 08, 2013 at 09:34:26AM +0100, Mark Brown wrote:
> On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote:
> > On Saturday 06 July 2013 14:01:12 Maxime Ripard wrote:
>
> > > > > > a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed property names
>
> > > > > > regmap = <&at25 0xstart 0xlen>;
> > > > > > regmap-names = "mac-address";
>
> > > > > > b) like gpio, regulator: variable property names
>
> > > > > > mac-storage = <&at25 0xstart 0xlen>;
>
> > > > > > It's unfortunate that we already have examples of both. They are largely
> > > > > > equivalent, but the tendency is towards the first.
>
> > > > > I don't have a strong feeling for one against another, so whatever works
> > > > > best. Both solutions will be a huge improvement anyway
>
> > > > > Just out of curiosity, is there any advantages besides having a fixed
> > > > > property name to the first solution?
>
> > > > I think it's mostly for consistency: trying to get most subsystems to
> > > > do it the same way to make it easier for people to write dts files.
>
> > > > A lesser point is that it simplifies the driver code if you don't
> > > > have to pass a name.
>
> On the other hand something with human readable names is much more
> legible if humans ever have to read or write the DT bindings. This
> mostly applies when there are many instances of the property (for
> example, many devices have lots of power supplies) or when some
> instances of the property are optional (for example, many devices can
> use GPIOs for many different functions but usually not all of them are
> connected and there's no particular order in which they might get
> connected).

I guess we would have only a few of these in our cases, so it doesn't
really qualify for the "many instances" I guess, but I do find the
GPIO/regulator-like properties more intuitive and more readable as
well.

> > > So that leave us with mainly one path to achieve this goal:
> > > - Add a regmap-mtd backend
> > > - Add DT parsing code for regmap
> > > - Move the EEPROM drivers from misc to mtd
>
> > Yes, I think that would be good. For the last step, we definitely need
> > buy-in from Wolfgand and Jean, as they are maintaining the current eeprom
> > drivers.
>
> I'd really like to see more discussion of this "DT parsing code for
> regmap" idea... I've missed almost all the context here.

The context was that I found we lack a way to simply express the need
for one driver to get a value from an EEPROM-like device, for example to
get a MAC Address, or a serial number, in a generic way, without having
to poke directly with some custom function that would be exported by the
EEPROM driver.

And the lack of infrastructure/framework to handle the EEPROM right now
only make the thing a bit harder.

What we've been discussing so far is that:
- To have a common framework we could base our work on, we could move
the EEPROM drivers from drivers/misc/eeprom to MTD
- To declare the ranges that needed to be used by a driver that was
needing a value from one of those MTD drivers, we would use regmap
with a MTD backend
- And since we actually need to declare which ranges and in which
device one driver would have to retrieve this value from, we were
actually in need of DT bindings.

This is pretty much the only context involved, and we are at the early
stage of the discussion, so any comment is very welcome :)

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (3.50 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-08 21:04:58

by Maxime Ripard

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Mon, Jul 08, 2013 at 09:36:14AM +0100, Mark Brown wrote:
> On Sun, Jul 07, 2013 at 09:15:01AM +0200, Maxime Ripard wrote:
> > On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote:
>
> > > We also have a bunch of OTP drivers spread around the kernel, it probably
> > > makes sense to consolidate them at the same time, at least on the DT binding
> > > side if not the device drivers.
>
> > From a quick grep, the only one I've seen so far are:
> > - imx6q, that has a hook at machine start to poke into its OCOTP to
> > retrieve some frequency scaling parameters it seems. I'm not sure
> > how the current solution could improve the situation for this
> > use-case, but the DT bindings of the OCOTP is just a DT node, with
> > no clients, so we have nothing to worry about here.
> > - imx28, that has a hook at machine start to look up the MAC address
> > values and patch the ethernet controller nodes to add the right
> > local-mac-address property. This one could benefit from the new
> > bindings, but we already mentionned it, and I intended to develop
> > with an imx28 board anyway.
> > - picoxcell-pc3x3 DTSI has a node for a OTP device, but they don't
> > seem to be doing anything with it, nor do they seem to have a driver
> > for it. So I guess we don't care about migrating for this one
> > either.
>
> > Did you have other cases in mind?
>
> We have some OTP support in the ab8500 and wm831x MFD drivers too but
> they just expose the data.

I guess you mean ab8100, right?

Anyway, thanks for pointing this out

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.67 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-09 14:56:08

by Mark Brown

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

On Mon, Jul 08, 2013 at 10:25:38PM +0200, Maxime Ripard wrote:
> On Mon, Jul 08, 2013 at 09:34:26AM +0100, Mark Brown wrote:

> > I'd really like to see more discussion of this "DT parsing code for
> > regmap" idea... I've missed almost all the context here.

> The context was that I found we lack a way to simply express the need
> for one driver to get a value from an EEPROM-like device, for example to
> get a MAC Address, or a serial number, in a generic way, without having
> to poke directly with some custom function that would be exported by the
> EEPROM driver.

This sort of information is often stored in places like flash partitions
too. Are we sure that regmap is a good place to be hooking in here?
The use case is sane, and being able to use regmap to do some of it
seems sensible (I've seen people use OTP in PMICs for similar purposes)
but perhaps an additional layer of abstraction on top makes sense.

> What we've been discussing so far is that:
> - To have a common framework we could base our work on, we could move
> the EEPROM drivers from drivers/misc/eeprom to MTD
> - To declare the ranges that needed to be used by a driver that was
> needing a value from one of those MTD drivers, we would use regmap
> with a MTD backend
> - And since we actually need to declare which ranges and in which
> device one driver would have to retrieve this value from, we were
> actually in need of DT bindings.

> This is pretty much the only context involved, and we are at the early
> stage of the discussion, so any comment is very welcome :)

If this stuff is being represented in MTD doesn't MTD already have
adequate abstractions for saying "this region in flash". But otherwise
this seems fine, it's not a generic regmap DT binding but instead rather
more specific than that.


Attachments:
(No filename) (1.78 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-07-11 17:05:21

by Maxime Ripard

[permalink] [raw]
Subject: Re: MTD EEPROM support and driver integration

Hi Mark,

On Tue, Jul 09, 2013 at 03:55:10PM +0100, Mark Brown wrote:
> On Mon, Jul 08, 2013 at 10:25:38PM +0200, Maxime Ripard wrote:
> > On Mon, Jul 08, 2013 at 09:34:26AM +0100, Mark Brown wrote:
>
> > > I'd really like to see more discussion of this "DT parsing code for
> > > regmap" idea... I've missed almost all the context here.
>
> > The context was that I found we lack a way to simply express the need
> > for one driver to get a value from an EEPROM-like device, for example to
> > get a MAC Address, or a serial number, in a generic way, without having
> > to poke directly with some custom function that would be exported by the
> > EEPROM driver.
>
> This sort of information is often stored in places like flash partitions
> too. Are we sure that regmap is a good place to be hooking in here?
> The use case is sane, and being able to use regmap to do some of it
> seems sensible (I've seen people use OTP in PMICs for similar purposes)
> but perhaps an additional layer of abstraction on top makes sense.

Ah, I didn't thought it could be stored into a partition. Ok, so using
an intermediate abstraction for this makes sense (probably using regmap
for all the accesses that are relevant, like i2c, spi or mmio)

> > What we've been discussing so far is that:
> > - To have a common framework we could base our work on, we could move
> > the EEPROM drivers from drivers/misc/eeprom to MTD
> > - To declare the ranges that needed to be used by a driver that was
> > needing a value from one of those MTD drivers, we would use regmap
> > with a MTD backend
> > - And since we actually need to declare which ranges and in which
> > device one driver would have to retrieve this value from, we were
> > actually in need of DT bindings.
>
> > This is pretty much the only context involved, and we are at the early
> > stage of the discussion, so any comment is very welcome :)
>
> If this stuff is being represented in MTD doesn't MTD already have
> adequate abstractions for saying "this region in flash". But otherwise
> this seems fine, it's not a generic regmap DT binding but instead rather
> more specific than that.

Yes, since we seem to be going to a point where regmap will be a
convenience in this case, we probably won't need a generic regmap
binding, but rather a generic way to define a range and offset into a
referenced device.

Arnd, the others, is this ok for you?

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (2.49 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments