Re: [U-Boot] [PATCH v2 19/26] dm: i2c: Move slave details to child platdata
Hi Masahiro,
On 23 January 2015 at 05:32, Masahiro Yamada wrote:
> Hi Simon,
>
>
>
> On Mon, 19 Jan 2015 20:12:48 -0700
> Simon Glass wrote:
>
>> if (offset_len > I2C_MAX_OFFSET_LEN)
>> return -EINVAL;
>> @@ -450,13 +448,26 @@ int i2c_post_bind(struct udevice *dev)
>> return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false);
>> }
>>
>> +int i2c_child_post_bind(struct udevice *dev)
>> +{
>> + struct dm_i2c_chip *plat = dev_get_parent_platdata(dev);
>> +
>> + if (dev->of_offset == -1)
>> + return 0;
>> +
>> + return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, plat);
>> +}
>> +
>
>
> Add "static" to i2c_post_bind() and i2c_child_post_bind().
>
>
>
>> UCLASS_DRIVER(i2c) = {
>> .id = UCLASS_I2C,
>> .name = "i2c",
>> .flags = DM_UC_FLAG_SEQ_ALIAS,
>> - .per_device_auto_alloc_size = sizeof(struct dm_i2c_bus),
>> .post_bind = i2c_post_bind,
>> .post_probe = i2c_post_probe,
>> + .per_device_auto_alloc_size = sizeof(struct dm_i2c_bus),
>> + .per_child_auto_alloc_size = sizeof(struct dm_i2c_chip),
>> + .per_child_platdata_auto_alloc_size = sizeof(struct dm_i2c_chip),
>> + .child_post_bind = i2c_child_post_bind,
>> };
>
>
> Now struct dm_i2c_chip is allocated on
> both .per_child_auto_alloc_size and .per_child_platdata_auto_alloc_size.
>
> The former is probably unused.
>
>
>
>
>
>> UCLASS_DRIVER(i2c_generic) = {
>> diff --git a/drivers/i2c/i2c-uniphier-f.c b/drivers/i2c/i2c-uniphier-f.c
>> index b0d30f7..6707edd 100644
>> --- a/drivers/i2c/i2c-uniphier-f.c
>> +++ b/drivers/i2c/i2c-uniphier-f.c
>> @@ -145,16 +145,6 @@ static int uniphier_fi2c_remove(struct udevice *dev)
>> return 0;
>> }
>>
>> -static int uniphier_fi2c_child_pre_probe(struct udevice *dev)
>> -{
>> - struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
>> -
>> - if (dev->of_offset == -1)
>> - return 0;
>> - return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
>> -i2c_chip);
>> -}
>> -
>
>
> Currently, i2c_chip_ofdata_to_platdata() is only used in i2c-uclass.c
>
> Perhaps it can become a "static" function.
> Or it might be useful to override something ?
I think it might still be useful to call it from a driver in some
cases. Certainly it is better than having the driver duplicate code.
On the other hand, I hope that the uclass can do this 'default'
processing and the driver can just add to it. We will see.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 19/26] dm: i2c: Move slave details to child platdata
Hi Simon,
On Mon, 19 Jan 2015 20:12:48 -0700
Simon Glass wrote:
> if (offset_len > I2C_MAX_OFFSET_LEN)
> return -EINVAL;
> @@ -450,13 +448,26 @@ int i2c_post_bind(struct udevice *dev)
> return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false);
> }
>
> +int i2c_child_post_bind(struct udevice *dev)
> +{
> + struct dm_i2c_chip *plat = dev_get_parent_platdata(dev);
> +
> + if (dev->of_offset == -1)
> + return 0;
> +
> + return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, plat);
> +}
> +
Add "static" to i2c_post_bind() and i2c_child_post_bind().
> UCLASS_DRIVER(i2c) = {
> .id = UCLASS_I2C,
> .name = "i2c",
> .flags = DM_UC_FLAG_SEQ_ALIAS,
> - .per_device_auto_alloc_size = sizeof(struct dm_i2c_bus),
> .post_bind = i2c_post_bind,
> .post_probe = i2c_post_probe,
> + .per_device_auto_alloc_size = sizeof(struct dm_i2c_bus),
> + .per_child_auto_alloc_size = sizeof(struct dm_i2c_chip),
> + .per_child_platdata_auto_alloc_size = sizeof(struct dm_i2c_chip),
> + .child_post_bind = i2c_child_post_bind,
> };
Now struct dm_i2c_chip is allocated on
both .per_child_auto_alloc_size and .per_child_platdata_auto_alloc_size.
The former is probably unused.
> UCLASS_DRIVER(i2c_generic) = {
> diff --git a/drivers/i2c/i2c-uniphier-f.c b/drivers/i2c/i2c-uniphier-f.c
> index b0d30f7..6707edd 100644
> --- a/drivers/i2c/i2c-uniphier-f.c
> +++ b/drivers/i2c/i2c-uniphier-f.c
> @@ -145,16 +145,6 @@ static int uniphier_fi2c_remove(struct udevice *dev)
> return 0;
> }
>
> -static int uniphier_fi2c_child_pre_probe(struct udevice *dev)
> -{
> - struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
> -
> - if (dev->of_offset == -1)
> - return 0;
> - return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
> -i2c_chip);
> -}
> -
Currently, i2c_chip_ofdata_to_platdata() is only used in i2c-uclass.c
Perhaps it can become a "static" function.
Or it might be useful to override something ?
Best Regards
Masahiro Yamada
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 19/26] dm: i2c: Move slave details to child platdata
At present we go through various contortions to store the I2C's chip
address in its private data. This only exists when the chip is active so
must be set up when it is probed. Until the device is probed we don't
actually record what address it will appear on.
However, now that we can support per-child platform data, we can use that
instead. This allows us to set up the address when the child is bound,
and avoid the messy contortions.
Unfortunately this is a fairly large change and it seems to be difficult to
break it down further.
Signed-off-by: Simon Glass
---
Changes in v2: None
drivers/i2c/i2c-uclass-compat.c | 2 +-
drivers/i2c/i2c-uclass.c| 53 +
drivers/i2c/i2c-uniphier-f.c| 12 --
drivers/i2c/i2c-uniphier.c | 12 --
drivers/i2c/s3c24x0_i2c.c | 12 --
drivers/i2c/sandbox_i2c.c | 28 +-
drivers/i2c/tegra_i2c.c | 18 --
include/i2c.h | 4 ++--
8 files changed, 41 insertions(+), 100 deletions(-)
diff --git a/drivers/i2c/i2c-uclass-compat.c b/drivers/i2c/i2c-uclass-compat.c
index d8de46a..223f238 100644
--- a/drivers/i2c/i2c-uclass-compat.c
+++ b/drivers/i2c/i2c-uclass-compat.c
@@ -20,7 +20,7 @@ static int i2c_compat_get_device(uint chip_addr, int alen,
ret = i2c_get_chip_for_busnum(cur_busnum, chip_addr, alen, devp);
if (ret)
return ret;
- chip = dev_get_parentdata(*devp);
+ chip = dev_get_parent_platdata(*devp);
if (chip->offset_len != alen) {
printf("I2C chip %x: requested alen %d does not match chip
offset_len %d\n",
chip_addr, alen, chip->offset_len);
diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index 94b49df..83c7a04 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -50,7 +50,7 @@ static int i2c_setup_offset(struct dm_i2c_chip *chip, uint
offset,
static int i2c_read_bytewise(struct udevice *dev, uint offset,
uint8_t *buffer, int len)
{
- struct dm_i2c_chip *chip = dev_get_parentdata(dev);
+ struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
struct udevice *bus = dev_get_parent(dev);
struct dm_i2c_ops *ops = i2c_get_ops(bus);
struct i2c_msg msg[2], *ptr;
@@ -79,7 +79,7 @@ static int i2c_read_bytewise(struct udevice *dev, uint offset,
static int i2c_write_bytewise(struct udevice *dev, uint offset,
const uint8_t *buffer, int len)
{
- struct dm_i2c_chip *chip = dev_get_parentdata(dev);
+ struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
struct udevice *bus = dev_get_parent(dev);
struct dm_i2c_ops *ops = i2c_get_ops(bus);
struct i2c_msg msg[1];
@@ -102,7 +102,7 @@ static int i2c_write_bytewise(struct udevice *dev, uint
offset,
int dm_i2c_read(struct udevice *dev, uint offset, uint8_t *buffer, int len)
{
- struct dm_i2c_chip *chip = dev_get_parentdata(dev);
+ struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
struct udevice *bus = dev_get_parent(dev);
struct dm_i2c_ops *ops = i2c_get_ops(bus);
struct i2c_msg msg[2], *ptr;
@@ -133,7 +133,7 @@ int dm_i2c_read(struct udevice *dev, uint offset, uint8_t
*buffer, int len)
int dm_i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer,
int len)
{
- struct dm_i2c_chip *chip = dev_get_parentdata(dev);
+ struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
struct udevice *bus = dev_get_parent(dev);
struct dm_i2c_ops *ops = i2c_get_ops(bus);
struct i2c_msg msg[1];
@@ -223,7 +223,7 @@ static int i2c_probe_chip(struct udevice *bus, uint
chip_addr,
static int i2c_bind_driver(struct udevice *bus, uint chip_addr, uint
offset_len,
struct udevice **devp)
{
- struct dm_i2c_chip chip;
+ struct dm_i2c_chip *chip;
char name[30], *str;
struct udevice *dev;
int ret;
@@ -236,11 +236,11 @@ static int i2c_bind_driver(struct udevice *bus, uint
chip_addr, uint offset_len,
goto err_bind;
/* Tell the device what we know about it */
- memset(&chip, '\0', sizeof(chip));
- chip.chip_addr = chip_addr;
- chip.offset_len = offset_len;
- ret = device_probe_child(dev, &chip);
- debug("%s: device_probe_child: ret=%d\n", __func__, ret);
+ chip = dev_get_parent_platdata(dev);
+ chip->chip_addr = chip_addr;
+ chip->offset_len = offset_len;
+ ret = device_probe(dev);
+ debug("%s: device_probe: ret=%d\n", __func__, ret);
if (ret)
goto err_probe;
@@ -248,6 +248,10 @@ static int i2c_bind_driver(struct udevice *bus, uint
chip_addr, uint offset_len,
return 0;
err_probe:
+ /*
+* If the device failed to probe, unbind it. There is nothing there
+
