[RFC] at24: get devfs name from dt aliase

[RFC] at24: get devfs name from dt aliase

[Antony Pavlov]

At the moment barebox can't work correctly with more
that one at24 eeprom because drivers/eeprom/at24.c
tries to register all eeproms as /dev/eeprom0.

I have tested this on a system with three at24 eeproms,
please see this dts-file fragment:

    &i2c0 {
        status = "okay";
    
        i2c-switch@70 {
            compatible = "nxp,pca9545";
            #address-cells = <1>;
            #size-cells = <0>;
            i2c-parent = <&i2c0>;
            reg = <0x70>;
    
            i2c@1 { /* misc devices */
                reg = <1>;
    
                eeprom: at24@50 {
                    compatible = "at24,24c1024";
                    reg = <0x50>;
                };
            };
    
            i2c@2 { /* SPD */
                reg = <2>;
    
                spd: at24@53 {
                    compatible = "at24,spd";
                    reg = <0x53>;
                };
            };
    
            i2c@3 { /* DVI/HDMI */
                reg = <3>;
    
                ddc: at24@50 {
                    compatible = "at24,24c02";
                    reg = <0x50>;
                };
            };
        };
    };

A possible solution is using aliases for naming devices
under dev/, e.g.:

    / {
        aliases {
            eeprom = &eeprom;
            spd = &spd;
            ddc = &ddc;
        };
    }

So we'll get these files in the /dev directory:

    crw-------            256 /dev/ddc
    crw-------         131072 /dev/eeprom
    cr--------            256 /dev/spd

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 drivers/eeprom/at24.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/eeprom/at24.c b/drivers/eeprom/at24.c
index 3c0a7a9..85fb035 100644
--- a/drivers/eeprom/at24.c
+++ b/drivers/eeprom/at24.c
@@ -379,6 +379,7 @@ static int at24_probe(struct device_d *dev)
 	struct at24_data *at24;
 	int err;
 	unsigned i, num_addresses;
+	const char *eeprom_name = NULL;
 
 	if (dev->platform_data) {
 		chip = *(struct at24_platform_data *)dev->platform_data;
@@ -429,7 +430,17 @@ static int at24_probe(struct device_d *dev)
 
 	at24->chip = chip;
 	at24->num_addresses = num_addresses;
-	at24->cdev.name = asprintf("eeprom%d", dev->id);
+
+	if (dev->device_node) {
+		eeprom_name = of_alias_get(dev->device_node);
+	}
+
+	if (!eeprom_name) {
+		at24->cdev.name = asprintf("eeprom%d", dev->id);
+	} else {
+		at24->cdev.name = eeprom_name;
+	}
+
 	at24->cdev.priv = at24;
 	at24->cdev.dev = dev;
 	at24->cdev.ops = &at24->fops;
-- 
2.6.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

RE: [RFC] at24: get devfs name from dt aliase

[Trent Piepho]

> -----Original Message-----
> From: Antony Pavlov [mailto:antonynpavlov@gmail.com]
> 
> At the moment barebox can't work correctly with more that one at24
> eeprom because drivers/eeprom/at24.c tries to register all eeproms as
> /dev/eeprom0.

I had this same problem with my system.  It is because different types of EEPROMs have different names and the IDs are assigned sequentially for each different name.  So if you have two 24c02 and one 24c1025, they would have the ids 0, 1, and 0.  The code that produces the IDs for the i2c devices (24c020, 24c021, 2401250) does not know that the eeprom driver will try to make cdevs with the same name pattern from all of them.

I did a different solution, see below, but didn't send it out as I'm not happy with it.  I thought also of an alias like this, but feel like there are also problems with that approach.

1. It seems like it should not be necessary to add aliases to get the system to work at all.
2. My system also has <64 kB xloader which does not use OF to save space.  So alias will not work.  I don't know of an alternative to the alias system when using i2c_register_board_info().

Subject: [PATCH] eeprom: Number eeproms sequentially

eeprom cdevs were being enumerated using the id of the underlying
device.  The problem with this is that the I2C core assigns IDs
separately for each different type of device.  So if one has two
different types of eeprom both will start with IDs of zero.  This
results in cdev names that conflict.

So just assign IDs sequentially to eeproms as they are probed.
---
 drivers/eeprom/at24.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/eeprom/at24.c b/drivers/eeprom/at24.c
index 23c3faa..8121b79 100644
--- a/drivers/eeprom/at24.c
+++ b/drivers/eeprom/at24.c
@@ -356,6 +356,7 @@ static int at24_probe(struct device_d *dev)
        struct at24_data *at24;
        int err;
        unsigned i, num_addresses;
+       static int eeprom_id = 0;

        if (dev->platform_data) {
                chip = *(struct at24_platform_data *)dev->platform_data;
@@ -406,7 +407,7 @@ static int at24_probe(struct device_d *dev)

        at24->chip = chip;
        at24->num_addresses = num_addresses;
-       at24->cdev.name = asprintf("eeprom%d", dev->id);
+       at24->cdev.name = asprintf("eeprom%d", eeprom_id++);
        at24->cdev.priv = at24;
        at24->cdev.dev = dev;
        at24->cdev.ops = &at24->fops;
--
1.8.3.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC] at24: get devfs name from dt aliase

[Sascha Hauer]

On Fri, Nov 27, 2015 at 08:37:02AM +0000, Trent Piepho wrote:
> > -----Original Message-----
> > From: Antony Pavlov [mailto:antonynpavlov@gmail.com]
> > 
> > At the moment barebox can't work correctly with more that one at24
> > eeprom because drivers/eeprom/at24.c tries to register all eeproms as
> > /dev/eeprom0.
> 
> I had this same problem with my system.  It is because different types of EEPROMs have different names and the IDs are assigned sequentially for each different name.  So if you have two 24c02 and one 24c1025, they would have the ids 0, 1, and 0.  The code that produces the IDs for the i2c devices (24c020, 24c021, 2401250) does not know that the eeprom driver will try to make cdevs with the same name pattern from all of them.
> 
> I did a different solution, see below, but didn't send it out as I'm not happy with it.  I thought also of an alias like this, but feel like there are also problems with that approach.
> 
> 1. It seems like it should not be necessary to add aliases to get the system to work at all.

Antonys patch falls back to dynamic numbering if no alias is found.

> 2. My system also has <64 kB xloader which does not use OF to save
> space.  So alias will not work.  I don't know of an alternative to the
> alias system when using i2c_register_board_info().

We also have the approach of putting the name into platform_data, this
is done with some MMC controllers. However, with an xloader wouldn't it
be possible to just register the device which you need? You don't need
all EEPROMs in the xloader, right?

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC] at24: get devfs name from dt aliase

[Trent Piepho]

On Mon, 2015-11-30 at 08:16 +0100, Sascha Hauer wrote:
> On Fri, Nov 27, 2015 at 08:37:02AM +0000, Trent Piepho wrote:
> > > -----Original Message-----
> > > From: Antony Pavlov [mailto:antonynpavlov@gmail.com]
> > > 
> > > At the moment barebox can't work correctly with more that one at24
> > > eeprom because drivers/eeprom/at24.c tries to register all eeproms as
> > > /dev/eeprom0.
> > 
> > I had this same problem with my system.  It is because different types of EEPROMs have different names and the IDs are assigned sequentially for each different name.  So if you have two 24c02 and one 24c1025, they would have the ids 0, 1, and 0.  The code that produces the IDs for the i2c devices (24c020, 24c021, 2401250) does not know that the eeprom driver will try to make cdevs with the same name pattern from all of them.
> > 
> > I did a different solution, see below, but didn't send it out as I'm not happy with it.  I thought also of an alias like this, but feel like there are also problems with that approach.
> > 
> > 1. It seems like it should not be necessary to add aliases to get the system to work at all.
> 
> Antonys patch falls back to dynamic numbering if no alias is found.

But still some eeproms will not show up with no error message.  It seems
like dynamic numbering should work better than that.

> > 2. My system also has <64 kB xloader which does not use OF to save
> > space.  So alias will not work.  I don't know of an alternative to the
> > alias system when using i2c_register_board_info().
> 
> We also have the approach of putting the name into platform_data, this
> is done with some MMC controllers. However, with an xloader wouldn't it
> be possible to just register the device which you need? You don't need
> all EEPROMs in the xloader, right?

Remains to be seen.  At this point of three types on the board, I'm
using two in the xloader.  It could change with further development.

I suppose adding the name to the platform data would be the best way to
get OF alias like naming.  Though I'd be fine with just dynamic
numbering that works, hence my simple patch to add that.


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox