From: Bartosz Golaszewski <redacted>
Since I took over maintainership of the at24 driver I've been working
towards removing at24_platform_data in favor for device properties.
DaVinci is the only platform that's still using it - all other users
have already been converted.
One of the obstacles in case of DaVinci is removing the setup() callback
from the pdata struct, the only user of which are some davinci boards.
Most boards use the EEPROM to store the MAC address. This series adds
support for cell lookups to the nvmem framework, registers relevant
cells for all users, converts the davinci_emac driver to using them
and replaces at24_platform_data with device properties.
The only board that's still using this callback is now mityomapl138.
Unfortunately it stores more info in EEPROM than just the MAC address
and will require some more work. Unfortunately I don't have access
to this board so I can't test any actual solutions on a live hardware.
Tested on a dm365-evm board.
Bartosz Golaszewski (14):
nvmem: add support for cell lookups
ARM: davinci: dm365-evm: use nvmem lookup for mac address
ARM: davinci: dm644-evm: use nvmem lookup for mac address
ARM: davinci: dm646x-evm: use nvmem lookup for mac address
ARM: davinci: da830-evm: use nvmem lookup for mac address
ARM: davinci: mityomapl138: add nvmem cells lookup entries
net: davinci_emac: use nvmem to retrieve the mac address
ARM: davinci: mityomapl138: don't read the MAC address from machine
code
ARM: davinci: dm365-evm: use device properties for at24 eeprom
ARM: davinci: da830-evm: use device properties for at24 eeprom
ARM: davinci: dm644x-evm: use device properties for at24 eeprom
ARM: davinci: dm646x-evm: use device properties for at24 eeprom
ARM: davinci: sffsdr: fix the at24 eeprom device name
ARM: davinci: sffsdr: use device properties for at24 eeprom
arch/arm/mach-davinci/board-da830-evm.c | 25 +++++++---
arch/arm/mach-davinci/board-dm365-evm.c | 25 +++++++---
arch/arm/mach-davinci/board-dm644x-evm.c | 24 ++++++---
arch/arm/mach-davinci/board-dm646x-evm.c | 25 +++++++---
arch/arm/mach-davinci/board-mityomapl138.c | 30 +++++++++---
arch/arm/mach-davinci/board-sffsdr.c | 13 +++--
drivers/net/ethernet/ti/davinci_emac.c | 29 +++++++----
drivers/nvmem/core.c | 57 +++++++++++++++++++++-
include/linux/nvmem-consumer.h | 6 +++
include/linux/nvmem-provider.h | 6 +++
10 files changed, 182 insertions(+), 58 deletions(-)
--
2.17.1
From: Bartosz Golaszewski <redacted>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/board-dm646x-evm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
From: Bartosz Golaszewski <redacted>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/board-sffsdr.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
From: Bartosz Golaszewski <redacted>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/board-dm644x-evm.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
From: Bartosz Golaszewski <redacted>
The currently used 24lc64 i2c device name doesn't match against any
of the devices supported by the at24 driver. Change it to the closest
compatible chip.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/board-sffsdr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Bartosz Golaszewski <redacted>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/board-da830-evm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
From: Bartosz Golaszewski <redacted>
We want to work towards phasing out the at24_platform_data structure.
There are few users and its contents can be represented using generic
device properties. Using device properties only will allow us to
significantly simplify the at24 configuration code.
Remove the at24_platform_data structure and replace it with an array
of property entries. Drop the byte_len/size property, as the model name
already implies the EEPROM's size.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/board-dm365-evm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
From: Bartosz Golaszewski <redacted>
This is now done by the emac driver using a registered nvmem cell.
Signed-off-by: Bartosz Golaszewski <redacted>
---
arch/arm/mach-davinci/board-mityomapl138.c | 8 --------
1 file changed, 8 deletions(-)
@@ -120,7 +120,6 @@ static void read_factory_config(struct nvmem_device *nvmem, void *context){intret;constchar*partnum=NULL;-structdavinci_soc_info*soc_info=&davinci_soc_info;if(!IS_BUILTIN(CONFIG_NVMEM)){pr_warn("Factory Config not available without CONFIG_NVMEM\n");
@@ -146,13 +145,6 @@ static void read_factory_config(struct nvmem_device *nvmem, void *context)gotobad_config;}-pr_info("Found MAC = %pM\n",factory_config.mac);-if(is_valid_ether_addr(factory_config.mac))-memcpy(soc_info->emac_pdata->mac_addr,-factory_config.mac,ETH_ALEN);-else-pr_warn("Invalid MAC found in factory config block\n");-partnum=factory_config.partnum;pr_info("Part Number = %s\n",partnum);
From: Bartosz Golaszewski <redacted>
All users which store the MAC address in EEPROM now register relevant
nvmem cells. Switch to retrieving the MAC address over the nvmem
framework.
Signed-off-by: Bartosz Golaszewski <redacted>
---
drivers/net/ethernet/ti/davinci_emac.c | 29 +++++++++++++++++---------
1 file changed, 19 insertions(+), 10 deletions(-)
@@ -1783,7 +1776,9 @@ static int davinci_emac_probe(struct platform_device *pdev)structcpdma_paramsdma_params;structclk*emac_clk;unsignedlongemac_bus_frequency;-+structnvmem_cell*cell;+void*mac_addr;+size_tmac_addr_len;/* obtain emac clock from kernel */emac_clk=devm_clk_get(&pdev->dev,NULL);
@@ -1815,8 +1810,22 @@ static int davinci_emac_probe(struct platform_device *pdev)gotoerr_free_netdev;}+cell=nvmem_cell_get(&pdev->dev,"mac-address");+if(!IS_ERR(cell)){+mac_addr=nvmem_cell_read(cell,&mac_addr_len);+if(!IS_ERR(mac_addr)){+if(is_valid_ether_addr(mac_addr)){+dev_info(&pdev->dev,+"Read MAC addr from EEPROM: %pM\n",+mac_addr);+memcpy(priv->mac_addr,mac_addr,ETH_ALEN);+}+kfree(mac_addr);+}+nvmem_cell_put(cell);+}+/* MAC addr and PHY mask , RMII enable info from platform_data */-memcpy(priv->mac_addr,pdata->mac_addr,ETH_ALEN);priv->phy_id=pdata->phy_id;priv->rmii_en=pdata->rmii_en;priv->version=pdata->version;
On 06/25/2018 10:50 AM, Bartosz Golaszewski wrote:
quoted hunk
From: Bartosz Golaszewski <redacted>
All users which store the MAC address in EEPROM now register relevant
nvmem cells. Switch to retrieving the MAC address over the nvmem
framework.
Signed-off-by: Bartosz Golaszewski <redacted>
---
drivers/net/ethernet/ti/davinci_emac.c | 29 +++++++++++++++++---------
1 file changed, 19 insertions(+), 10 deletions(-)
Not sure it is correct. of_get_mac_address() is called when board data doesn't
provide MAC address with expectation that MAC address is defined in DT (usually by u-boot).
Standard DT properties for MAC address "local-mac-address" and "mac-address".
So, you can't just drop it and replace with nvmem.
Two options are here:
1) try to read MAC-address from nvmem then try DT (as it's now)
2) try to read DT then try nvmem [then random MAC | fail]
From: Bartosz Golaszewski <redacted>
We can currently only register nvmem cells from device tree or by
manually calling nvmem_add_cells(). The latter options however forces
users to make sure that the nvmem provider with which the cells are
associated is registered before the call.
This patch proposes a new solution inspired by other frameworks that
offer resource lookups (GPIO, PWM etc.). It adds a function that allows
machine code to register nvmem lookup which are later lazily used to
add corresponding nvmem cells.
Signed-off-by: Bartosz Golaszewski <redacted>
---
drivers/nvmem/core.c | 57 +++++++++++++++++++++++++++++++++-
include/linux/nvmem-consumer.h | 6 ++++
include/linux/nvmem-provider.h | 6 ++++
3 files changed, 68 insertions(+), 1 deletion(-)
From: Andrew Lunn <andrew@lunn.ch> Date: 2018-06-25 17:41:17
On Mon, Jun 25, 2018 at 05:50:11PM +0200, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <redacted>
Since I took over maintainership of the at24 driver I've been working
towards removing at24_platform_data in favor for device properties.
DaVinci is the only platform that's still using it - all other users
have already been converted.
One of the obstacles in case of DaVinci is removing the setup() callback
from the pdata struct, the only user of which are some davinci boards.
Hi Bartosz
Nice code.
I've got a platform i want to add sometime soon using the at24. I
noticed you doing the cleanup, so i avoided the setup() call. But
using it would of helped...
My platform is x86 based, so no device tree. I instantiate a number of
AT24 devices from a platform driver, and then add nvmem cells so i can
access data in these eeproms. This new code will make this simpler.
The only board that's still using this callback is now mityomapl138.
Unfortunately it stores more info in EEPROM than just the MAC address
and will require some more work. Unfortunately I don't have access
to this board so I can't test any actual solutions on a live hardware.
Depending on what i find in the EEPROM, i need to instantiate other
i2c devices. So i have the problem of knowing when the EEPROM has
actually probed and i can use the nvmem API to retrieve the contents.
What i have done so far, is registers a bus notifier on i2c_bus_type,
and look for BUS_NOTIFY_BOUND_DRIVER. I can then check if the i2c
client in the notifier is the at24 client. But when i then add more
i2c clients from inside the notifier i get lockdep splats. They are
false positives, but it does suggest it is not a good idea to do this.
So it would be good to have some sort of recommended alternative to
the setup() callback. Ideally it would be specific to a particular
at24, and safe to call other i2c functions from.
Do you have any ideas?
Andrew
2018-06-25 19:40 GMT+02:00 Andrew Lunn [off-list ref]:
On Mon, Jun 25, 2018 at 05:50:11PM +0200, Bartosz Golaszewski wrote:
quoted
From: Bartosz Golaszewski <redacted>
Since I took over maintainership of the at24 driver I've been working
towards removing at24_platform_data in favor for device properties.
DaVinci is the only platform that's still using it - all other users
have already been converted.
One of the obstacles in case of DaVinci is removing the setup() callback
from the pdata struct, the only user of which are some davinci boards.
Hi Bartosz
Nice code.
I've got a platform i want to add sometime soon using the at24. I
noticed you doing the cleanup, so i avoided the setup() call. But
using it would of helped...
My platform is x86 based, so no device tree. I instantiate a number of
AT24 devices from a platform driver, and then add nvmem cells so i can
access data in these eeproms. This new code will make this simpler.
quoted
The only board that's still using this callback is now mityomapl138.
Unfortunately it stores more info in EEPROM than just the MAC address
and will require some more work. Unfortunately I don't have access
to this board so I can't test any actual solutions on a live hardware.
Depending on what i find in the EEPROM, i need to instantiate other
i2c devices. So i have the problem of knowing when the EEPROM has
actually probed and i can use the nvmem API to retrieve the contents.
What i have done so far, is registers a bus notifier on i2c_bus_type,
and look for BUS_NOTIFY_BOUND_DRIVER. I can then check if the i2c
client in the notifier is the at24 client. But when i then add more
i2c clients from inside the notifier i get lockdep splats. They are
false positives, but it does suggest it is not a good idea to do this.
So it would be good to have some sort of recommended alternative to
the setup() callback. Ideally it would be specific to a particular
at24, and safe to call other i2c functions from.
Do you have any ideas?
Andrew
With my patch 1/14 you'll get -EPROBE_DEFER from nvmem_cell_get() if
the nvmem provider is not yet registered. Will that help in your case?
Bart
From: Andrew Lunn <andrew@lunn.ch> Date: 2018-06-25 18:03:38
With my patch 1/14 you'll get -EPROBE_DEFER from nvmem_cell_get() if
the nvmem provider is not yet registered. Will that help in your case?
I don't think so. My driver instantiates the AT24 device. So if i get
-EPROBE_DEFER, i need to cleanup the probe, and return -EPROBDE_DEFER
to the code. Which means i need to remove the AT24 device...
Andrew
2018-06-25 20:02 GMT+02:00 Andrew Lunn [off-list ref]:
quoted
With my patch 1/14 you'll get -EPROBE_DEFER from nvmem_cell_get() if
the nvmem provider is not yet registered. Will that help in your case?
I don't think so. My driver instantiates the AT24 device. So if i get
-EPROBE_DEFER, i need to cleanup the probe, and return -EPROBDE_DEFER
to the code. Which means i need to remove the AT24 device...
Andrew
Are you sure this is the correct approach? I understand that you want
to use something like board files for your machine? Wouldn't it be
better to register a platform device for at24, register a cell lookup
and then depend on that cell from your driver?
Would you mind describing your platform in detail a bit more?
Best regards,
Bartosz
From: Andrew Lunn <andrew@lunn.ch> Date: 2018-06-26 08:39:02
On Tue, Jun 26, 2018 at 09:44:19AM +0200, Bartosz Golaszewski wrote:
2018-06-25 20:02 GMT+02:00 Andrew Lunn [off-list ref]:
quoted
quoted
With my patch 1/14 you'll get -EPROBE_DEFER from nvmem_cell_get() if
the nvmem provider is not yet registered. Will that help in your case?
I don't think so. My driver instantiates the AT24 device. So if i get
-EPROBE_DEFER, i need to cleanup the probe, and return -EPROBDE_DEFER
to the code. Which means i need to remove the AT24 device...
Andrew
Are you sure this is the correct approach? I understand that you want
to use something like board files for your machine? Wouldn't it be
better to register a platform device for at24, register a cell lookup
and then depend on that cell from your driver?
Hi Bartosz
The machine is based around a Kontron Com Express module, with an
intel Ivy Bridge CPU. This is then placed into a custom carrier board,
which has a number of i2c devices.
I have a platform driver which matches on the DMI system ID for the
Kontron module.
The Com Express module has a PLD which implements i2c, gpio,
etc. There is an MFD for this, which instantiates the i2c-kempld i2c
bus driver.
My platform driver finds this i2c-kempld bus driver. If it does not
exist yet, it return -EPROBE_DEFER. If it is found it instantiates an
at24 device on it. I need to look at the content of the EEPROM to
determine the hardware revision, plus do a checksum. From that, i
need to instantiates 1 or 2 additional AT24, up to 4 GPIO expanders,
and i2c to spi converter, add some gpio-leds on the gpio expanders,
create a bit-banging MDIO bus, instantiate an Ethernet switch on the
MDIO bus, maybe add an Ethernet switch to the SPI bus, etc.
As you can see, i have a chain of events. I cannot move onto the next
part of the chain until i know the probe for the previous part has
finished. e.g. i cannot add gpio-leds until i know the gpio expander
has probed. But the gpio expander provides a call back, similar to the
at24 setup(). The MDIO bus and the SPI bus has a mechanism to register
an info structure, just like you have done for NVMEM cells. So that
works out.
The weak link in this chain is that first at24 probe, and knowing when
i can access the nvmem cells for the revision and checksum
information. -EPROBE_DEFER does not help me here. I need either some
sort of blocking wait for the cells to become available, or a callback
in a context which allows me to instantiate more devices.
I also have some steps which cannot be undone. You don't provide a
mechanism to unregister the nvmem info structure. The I2C and MDIO
equivalent also does not provide an unregister for bus info. So once i
register the first info structure, i'm past the point of no return. I
cannot return -EPROBE_DEFER because i cannot unregister the info
structures, so that i can register them again the next time the
platform driver gets probed.
Andrew
2018-06-26 10:38 GMT+02:00 Andrew Lunn [off-list ref]:
On Tue, Jun 26, 2018 at 09:44:19AM +0200, Bartosz Golaszewski wrote:
quoted
2018-06-25 20:02 GMT+02:00 Andrew Lunn [off-list ref]:
quoted
quoted
With my patch 1/14 you'll get -EPROBE_DEFER from nvmem_cell_get() if
the nvmem provider is not yet registered. Will that help in your case?
I don't think so. My driver instantiates the AT24 device. So if i get
-EPROBE_DEFER, i need to cleanup the probe, and return -EPROBDE_DEFER
to the code. Which means i need to remove the AT24 device...
Andrew
Are you sure this is the correct approach? I understand that you want
to use something like board files for your machine? Wouldn't it be
better to register a platform device for at24, register a cell lookup
and then depend on that cell from your driver?
Hi Bartosz
The machine is based around a Kontron Com Express module, with an
intel Ivy Bridge CPU. This is then placed into a custom carrier board,
which has a number of i2c devices.
I have a platform driver which matches on the DMI system ID for the
Kontron module.
The Com Express module has a PLD which implements i2c, gpio,
etc. There is an MFD for this, which instantiates the i2c-kempld i2c
bus driver.
My platform driver finds this i2c-kempld bus driver. If it does not
exist yet, it return -EPROBE_DEFER. If it is found it instantiates an
at24 device on it. I need to look at the content of the EEPROM to
determine the hardware revision, plus do a checksum. From that, i
need to instantiates 1 or 2 additional AT24, up to 4 GPIO expanders,
and i2c to spi converter, add some gpio-leds on the gpio expanders,
create a bit-banging MDIO bus, instantiate an Ethernet switch on the
MDIO bus, maybe add an Ethernet switch to the SPI bus, etc.
As you can see, i have a chain of events. I cannot move onto the next
part of the chain until i know the probe for the previous part has
finished. e.g. i cannot add gpio-leds until i know the gpio expander
has probed. But the gpio expander provides a call back, similar to the
at24 setup(). The MDIO bus and the SPI bus has a mechanism to register
an info structure, just like you have done for NVMEM cells. So that
works out.
The weak link in this chain is that first at24 probe, and knowing when
i can access the nvmem cells for the revision and checksum
information. -EPROBE_DEFER does not help me here. I need either some
sort of blocking wait for the cells to become available, or a callback
in a context which allows me to instantiate more devices.
I also have some steps which cannot be undone. You don't provide a
mechanism to unregister the nvmem info structure. The I2C and MDIO
equivalent also does not provide an unregister for bus info. So once i
register the first info structure, i'm past the point of no return. I
cannot return -EPROBE_DEFER because i cannot unregister the info
structures, so that i can register them again the next time the
platform driver gets probed.
Andrew
I see. I see it this way: the setup callback comes from the time when
we didn't have nvmem and should go away. I will protest loud whenever
someone will try to use it again and will work towards removing it as
soon as possible.
I will give your problem a thought and will try to get back with some
proposals - maybe we should, as you suggested, extend nvmem even
further to allow to remove nvmem info entries etc.
Best regards,
Bartosz Golaszewski
From: Andrew Lunn <andrew@lunn.ch> Date: 2018-06-26 13:22:32
I see. I see it this way: the setup callback comes from the time when
we didn't have nvmem and should go away. I will protest loud whenever
someone will try to use it again and will work towards removing it as
soon as possible.
The setup() callback could be moved into the nvmem framework, rather
than in the at24 driver. Make the call when the cells have been
connected to the backing store.
I will give your problem a thought and will try to get back with some
proposals - maybe we should, as you suggested, extend nvmem even
further to allow to remove nvmem info entries etc.
That does not help me too much. I have the same problem with i2c and
MDIO. So i actually prefer to keep this the same as all others.
Andrew
2018-06-26 15:21 GMT+02:00 Andrew Lunn [off-list ref]:
quoted
I see. I see it this way: the setup callback comes from the time when
we didn't have nvmem and should go away. I will protest loud whenever
someone will try to use it again and will work towards removing it as
soon as possible.
The setup() callback could be moved into the nvmem framework, rather
than in the at24 driver. Make the call when the cells have been
connected to the backing store.
This would at least make it more generic. And maybe I could also get
rid of the setup callback from the mityomapl138 board file.
Bart
quoted
I will give your problem a thought and will try to get back with some
proposals - maybe we should, as you suggested, extend nvmem even
further to allow to remove nvmem info entries etc.
That does not help me too much. I have the same problem with i2c and
MDIO. So i actually prefer to keep this the same as all others.
Andrew
2018-06-26 15:21 GMT+02:00 Andrew Lunn [off-list ref]:
quoted
quoted
I see. I see it this way: the setup callback comes from the time when
we didn't have nvmem and should go away. I will protest loud whenever
someone will try to use it again and will work towards removing it as
soon as possible.
The setup() callback could be moved into the nvmem framework, rather
than in the at24 driver. Make the call when the cells have been
connected to the backing store.
This would at least make it more generic. And maybe I could also get
rid of the setup callback from the mityomapl138 board file.
Bart
quoted
quoted
I will give your problem a thought and will try to get back with some
proposals - maybe we should, as you suggested, extend nvmem even
further to allow to remove nvmem info entries etc.
That does not help me too much. I have the same problem with i2c and
MDIO. So i actually prefer to keep this the same as all others.
Andrew
Hi Andrew,
MTD subsystem has the struct mtd_notifier and register_mtd_user(). I'm
wondering if we could implement something like this as a generic nvmem
functionality.
What do you think?
Bart