All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* platform_data in i2c device drivers
@ 2014-03-19 22:32 Frank Bormann
       [not found] ` <532A1B12.8080400-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Frank Bormann @ 2014-03-19 22:32 UTC (permalink / raw
  To: Rodolfo Giometti, Laurent Pinchart; +Cc: Linux I2C List

Hi Everyone,

I am looking at the i2c_pca954x i2c bus mux driver in linux-stable. My goal is 
to have the slave buses show up in Linux with static bus numbers. Ideally, I 
want to define the first bus number to use in the dtb.

It seems, the driver has already some support for static bus numbers as its 
probe function checks for the existence of a struct pca954x_platform_data 
instance in client->dev.platform_data and pca954x_platform_mode struct it points 
to has a member adap_id that seems to be doing exactly that judging by its 
documentation. However, calls made to pca954x_probe always have to platform_data 
pointer being passed in through client set to null.

In addition to that, the recent addition to the driver of a reset gpio to be 
configured reads directly from the dtb in the probe function.

I am unsure about how to set this up properly. On one hand, platform_data is 
being passed in to the probe function, which seem to indicate, there may be some 
generic place in the i2c core code to set driver-specific configuration, on the 
other hand, the recent reset gpio addition to the driver seems to indicate that 
the driver's probe function is in fact the right place to read additional 
configuration from the dtb.

Any help is greatly appreciated.

Thanks,
Frank

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: platform_data in i2c device drivers
       [not found] ` <532A1B12.8080400-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
@ 2014-03-20  0:10   ` Laurent Pinchart
  2014-03-20 16:12     ` Frank Bormann
       [not found]     ` <534FF708.7040409@yahoo.com>
  0 siblings, 2 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-03-20  0:10 UTC (permalink / raw
  To: Frank Bormann; +Cc: Rodolfo Giometti, Linux I2C List

Hi Frank,

On Wednesday 19 March 2014 18:32:50 Frank Bormann wrote:
> Hi Everyone,
> 
> I am looking at the i2c_pca954x i2c bus mux driver in linux-stable. My goal
> is to have the slave buses show up in Linux with static bus numbers.
> Ideally, I want to define the first bus number to use in the dtb.
> 
> It seems, the driver has already some support for static bus numbers as its
> probe function checks for the existence of a struct pca954x_platform_data
> instance in client->dev.platform_data and pca954x_platform_mode struct it
> points to has a member adap_id that seems to be doing exactly that judging
> by its documentation. However, calls made to pca954x_probe always have to
> platform_data pointer being passed in through client set to null.
> 
> In addition to that, the recent addition to the driver of a reset gpio to be
> configured reads directly from the dtb in the probe function.
> 
> I am unsure about how to set this up properly. On one hand, platform_data is
> being passed in to the probe function, which seem to indicate, there may be
> some generic place in the i2c core code to set driver-specific
> configuration, on the other hand, the recent reset gpio addition to the
> driver seems to indicate that the driver's probe function is in fact the
> right place to read additional configuration from the dtb.
> 
> Any help is greatly appreciated.

You're looking at two different configuration mechanisms, which probably 
explains your confusion.

Platform data is the oldest mechanism used to pass configuration information 
to the driver. This is largely used through the Linux kernel on a wide range 
of architectures. The idea is to store device-specific configuration 
information in board files (as you mentioned DT I'll assume you're working on 
ARM, so that would be arch/arm/mach-*/board-*.c) using driver-specific 
structures and associate those structures with devices. Drivers can then 
retrieve the platform data at probe time and configure the device accordingly.

The way platform data is associated with devices depends on the bus type. For 
I2C the i2c_board_info structure, used to instantiate I2C devices in board 
code, has a void *platform_data field that can be set to point to a platform 
data structure. You can find an example of this in the i2c3_devices array in 
arch/arm/mach-shmobile/board-kzm9g.c.

Device tree (DT) is a newer mechanism to specify hardware configuration. 
Instead of relying on C board files that contain a mix a code and data, the 
platform is described in a tree-like structure of devices with properties 
associated to all those devices. That structure is called the device tree and 
is compiled as a binary that gets passed to the kernel at boot time. When 
using the device tree, drivers don't receive platform data anymore but are 
responsible for parsing the content of the device tree to read platform-
specific hardware configuration information. The content of device tree nodes 
is defined in documents called DT bindings that can be found in 
Documentation/devicetree/bindings/ (i2c/i2c-mux-pca954x.txt in this case).

A NULL platform_data pointer can then mean either that your platform boots 
using the device tree, or that the board file doesn't specify any platform 
data for the device in case of legacy (non-DT) boot. There's also a hybrid 
method that can be used to associated platform data declared in C files to DT 
nodes, but that's for special cases only and shouldn't be used for the 
pca954x.

This being said, if your platform uses the device tree, you shouldn't (in 
theory at least) need static I2C bus numbers. This is why there is no DT 
property similar to the platform data adap_id field defined in the pca954x DT 
bindings.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: platform_data in i2c device drivers
  2014-03-20  0:10   ` Laurent Pinchart
@ 2014-03-20 16:12     ` Frank Bormann
       [not found]       ` <532B1367.8050906-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
       [not found]     ` <534FF708.7040409@yahoo.com>
  1 sibling, 1 reply; 13+ messages in thread
From: Frank Bormann @ 2014-03-20 16:12 UTC (permalink / raw
  To: Laurent Pinchart; +Cc: Linux I2C List

Hi Laurent,

First of all, thank you so much for your detailed explanation. This really helps 
out a lot.

Your assumptions are correct, I am needed working on an ARM device and it uses 
device tree for driver configuration.

Just to confirm that I understand you correctly, what you're saying is that the 
platform_data and the device tree configuration mechanisms have an exclusive-or 
relationship, at least when it comes down to an individual driver, is that 
correct? And, if there is no platform_data provided, it is perfectly permissible 
for the probe function of the driver to make call to of_* functions to read its 
configuration? Provided that the kernel has Open Firmware and Device Tree 
support enabled of course.

Would it also acceptable for a driver's probe function to allocate memory for a 
platform_data instance and use the platform_data member in client->dev to store 
a pointer to it if no previous platform_data is available from the 
i2c_board_info configuration mechanism? Again provided of course that the memory 
is freed and the platform_data member in client->dev set back to null in the 
remove function of the driver.

Thank you,
Frank

On 19/03/14 08:10 PM, Laurent Pinchart wrote:

> You're looking at two different configuration mechanisms, which probably
> explains your confusion.
>
> Platform data is the oldest mechanism used to pass configuration information
> to the driver. This is largely used through the Linux kernel on a wide range
> of architectures. The idea is to store device-specific configuration
> information in board files (as you mentioned DT I'll assume you're working on
> ARM, so that would be arch/arm/mach-*/board-*.c) using driver-specific
> structures and associate those structures with devices. Drivers can then
> retrieve the platform data at probe time and configure the device accordingly.
>
> The way platform data is associated with devices depends on the bus type. For
> I2C the i2c_board_info structure, used to instantiate I2C devices in board
> code, has a void *platform_data field that can be set to point to a platform
> data structure. You can find an example of this in the i2c3_devices array in
> arch/arm/mach-shmobile/board-kzm9g.c.
>
> Device tree (DT) is a newer mechanism to specify hardware configuration.
> Instead of relying on C board files that contain a mix a code and data, the
> platform is described in a tree-like structure of devices with properties
> associated to all those devices. That structure is called the device tree and
> is compiled as a binary that gets passed to the kernel at boot time. When
> using the device tree, drivers don't receive platform data anymore but are
> responsible for parsing the content of the device tree to read platform-
> specific hardware configuration information. The content of device tree nodes
> is defined in documents called DT bindings that can be found in
> Documentation/devicetree/bindings/ (i2c/i2c-mux-pca954x.txt in this case).
>
> A NULL platform_data pointer can then mean either that your platform boots
> using the device tree, or that the board file doesn't specify any platform
> data for the device in case of legacy (non-DT) boot. There's also a hybrid
> method that can be used to associated platform data declared in C files to DT
> nodes, but that's for special cases only and shouldn't be used for the
> pca954x.
>
> This being said, if your platform uses the device tree, you shouldn't (in
> theory at least) need static I2C bus numbers. This is why there is no DT
> property similar to the platform data adap_id field defined in the pca954x DT
> bindings.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: platform_data in i2c device drivers
       [not found]       ` <532B1367.8050906-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
@ 2014-03-20 16:25         ` Ben Dooks
       [not found]           ` <532B1689.3080202-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Ben Dooks @ 2014-03-20 16:25 UTC (permalink / raw
  To: Frank Bormann; +Cc: Laurent Pinchart, Linux I2C List

On 20/03/14 17:12, Frank Bormann wrote:
> Hi Laurent,
>
> First of all, thank you so much for your detailed explanation. This
> really helps out a lot.
>
> Your assumptions are correct, I am needed working on an ARM device and
> it uses device tree for driver configuration.
>
> Just to confirm that I understand you correctly, what you're saying is
> that the platform_data and the device tree configuration mechanisms have
> an exclusive-or relationship, at least when it comes down to an
> individual driver, is that correct? And, if there is no platform_data
> provided, it is perfectly permissible for the probe function of the
> driver to make call to of_* functions to read its configuration?
> Provided that the kernel has Open Firmware and Device Tree support
> enabled of course.

Yes, dev->of_node should be set appropriately for devices enumerated
from the OF. This can then be used to read anything that the driver
core has not already found.

>
> Would it also acceptable for a driver's probe function to allocate
> memory for a platform_data instance and use the platform_data member in
> client->dev to store a pointer to it if no previous platform_data is
> available from the i2c_board_info configuration mechanism? Again
> provided of course that the memory is freed and the platform_data member
> in client->dev set back to null in the remove function of the driver.

I think client->dev should be avoided if at-all possible. Many
drivers keep their own local copy of platform data or the pointer
to it in their driver private information.

I tend to prefer not to alloc platform_data for OF if possible
as it avoids fragmenting the memory. devm_kzalloc() can also
be used to avoid having to remember to free the memory again.




-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: platform_data in i2c device drivers
       [not found]           ` <532B1689.3080202-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
@ 2014-03-20 17:16             ` Frank Bormann
       [not found]               ` <532B2273.8040004-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Frank Bormann @ 2014-03-20 17:16 UTC (permalink / raw
  To: Ben Dooks; +Cc: Laurent Pinchart, Linux I2C List

Hi Ben,

> I think client->dev should be avoided if at-all possible. Many
> drivers keep their own local copy of platform data or the pointer
> to it in their driver private information.

I was thinking about that. But then again, I'd either have to copy the 
client->dev.platform_data pointer over to the private data, if it is non-null, 
or I would have to use some variation of

pdata = client->dev.platform_data ? client->dev.platform_data : priv_pd;

every time I want to access the configuration. Guess, that's not so bad though.

Thanks,
Frank

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: platform_data in i2c device drivers
       [not found]               ` <532B2273.8040004-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
@ 2014-03-20 17:51                 ` Laurent Pinchart
  2014-03-21 15:55                   ` Frank Bormann
  0 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2014-03-20 17:51 UTC (permalink / raw
  To: Frank Bormann; +Cc: Ben Dooks, Linux I2C List

Hi Frank,

On Thursday 20 March 2014 13:16:35 Frank Bormann wrote:
> Hi Ben,
> 
> > I think client->dev should be avoided if at-all possible. Many
> > drivers keep their own local copy of platform data or the pointer
> > to it in their driver private information.
> 
> I was thinking about that. But then again, I'd either have to copy the
> client->dev.platform_data pointer over to the private data, if it is
> non-null, or I would have to use some variation of
> 
> pdata = client->dev.platform_data ? client->dev.platform_data : priv_pd;
> 
> every time I want to access the configuration. Guess, that's not so bad
> though.

Most drivers store a pointer to the platform data or a copy of the platform 
data in a per-device driver private structure. They populate that pointer or 
copy from the platform data pointer (in the legacy case) or from the device 
tree content. Outside of the probe function the driver thus only accesses its 
private pointer or copy.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: platform_data in i2c device drivers
  2014-03-20 17:51                 ` Laurent Pinchart
@ 2014-03-21 15:55                   ` Frank Bormann
       [not found]                     ` <532C60EC.3060405-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Frank Bormann @ 2014-03-21 15:55 UTC (permalink / raw
  To: Laurent Pinchart; +Cc: Ben Dooks, Linux I2C List

Hi Laurent,

Thank you again for explaining. One last question if that's okay.

If I have i2c_board_info being passed in to the probe function through the 
platform_data pointer and I also have the same, similar or maybe even 
conflicting configuration information in the DTB, is there any of the two that 
is supposed to take precedence or should that be an error case?

Thanks,
Frank

On 20/03/14 01:51 PM, Laurent Pinchart wrote:
> Hi Frank,
>
> On Thursday 20 March 2014 13:16:35 Frank Bormann wrote:
>> Hi Ben,
>>
>>> I think client->dev should be avoided if at-all possible. Many
>>> drivers keep their own local copy of platform data or the pointer
>>> to it in their driver private information.
>>
>> I was thinking about that. But then again, I'd either have to copy the
>> client->dev.platform_data pointer over to the private data, if it is
>> non-null, or I would have to use some variation of
>>
>> pdata = client->dev.platform_data ? client->dev.platform_data : priv_pd;
>>
>> every time I want to access the configuration. Guess, that's not so bad
>> though.
>
> Most drivers store a pointer to the platform data or a copy of the platform
> data in a per-device driver private structure. They populate that pointer or
> copy from the platform data pointer (in the legacy case) or from the device
> tree content. Outside of the probe function the driver thus only accesses its
> private pointer or copy.
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: platform_data in i2c device drivers
       [not found]                     ` <532C60EC.3060405-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
@ 2014-03-21 16:01                       ` Laurent Pinchart
  0 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-03-21 16:01 UTC (permalink / raw
  To: Frank Bormann; +Cc: Ben Dooks, Linux I2C List

Hi Frank,

On Friday 21 March 2014 11:55:24 Frank Bormann wrote:
> Hi Laurent,
> 
> Thank you again for explaining. One last question if that's okay.
> 
> If I have i2c_board_info being passed in to the probe function through the
> platform_data pointer and I also have the same, similar or maybe even
> conflicting configuration information in the DTB, is there any of the two
> that is supposed to take precedence or should that be an error case?

That shouldn't happen (except in very special cases using OF_DEV_AUXDATA, but 
you really should think twice before going that way). If the I2C device is 
instantiated in board code with i2c_board_info then there will be no DT node 
corresponding to the device (client->dev.of_node will be NULL). If the device 
is instead instantiated from DT there will be no platform data (client-
>dev.platform_data will be NULL).

> On 20/03/14 01:51 PM, Laurent Pinchart wrote:
> > On Thursday 20 March 2014 13:16:35 Frank Bormann wrote:
> >> Hi Ben,
> >> 
> >>> I think client->dev should be avoided if at-all possible. Many
> >>> drivers keep their own local copy of platform data or the pointer
> >>> to it in their driver private information.
> >> 
> >> I was thinking about that. But then again, I'd either have to copy the
> >> client->dev.platform_data pointer over to the private data, if it is
> >> non-null, or I would have to use some variation of
> >> 
> >> pdata = client->dev.platform_data ? client->dev.platform_data : priv_pd;
> >> 
> >> every time I want to access the configuration. Guess, that's not so bad
> >> though.
> > 
> > Most drivers store a pointer to the platform data or a copy of the
> > platform data in a per-device driver private structure. They populate that
> > pointer or copy from the platform data pointer (in the legacy case) or
> > from the device tree content. Outside of the probe function the driver
> > thus only accesses its private pointer or copy.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be specified in the dts
       [not found]       ` <534FF708.7040409-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
@ 2014-04-17 15:46         ` Laurent Pinchart
  2014-04-17 18:00           ` Laxman Dewangan
       [not found]           ` <53501564.1090607@yahoo.com>
  0 siblings, 2 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-04-17 15:46 UTC (permalink / raw
  To: Frank Bormann; +Cc: Rodolfo Giometti, Linux I2C List

Hi Frank,

On Thursday 17 April 2014 11:45:12 Frank Bormann wrote:
> Hi Everyone,
> 
> I know that Laurent said, fixed-bus-number configuration from device_tree
> shouldn't in theory be required for the pca954x mux buses. I noticed
> however, that bus enumeration has changed when migrating from a 3.8 to a
> 3.12 kernel, particularly when using cascaded mux chips - one of the mux
> buses of the first mux chip has a 2nd mux chip conntected to it. Since I
> have implemented it anyway, you may as well consider it for inclusion in
> the mainstream Linux kernel. Please find the patch below.

My question still holds, why do you need fixed bus numbers in the first place 
?

>  From dc4968005ad7211867ac344df958d7e0605910dd Mon Sep 17 00:00:00 2001
> From: Frank Bormann <fbormann-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
> Date: Thu, 17 Apr 2014 11:37:08 -0400
> Subject: [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be
>   specified in the dts
> 
> This patch will allow specific i2c bus numbers for the downstream mux
> buses to be specified in the dts in much of the same way it can be
> specified in i2c_board_info. If any bus number specified is already in
> use by another i2c bus or fewer bus numbers are specified than
> downstream mux buses are available on the particular pca954x mux chip,
> the driver will fail to load.
> 
> dts example:
> fixed-bus-numbers = <3 4 5 6>;
> 
> Signed-off-by: Frank Bormann <fbormann-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
> ---
>   drivers/i2c/muxes/i2c-mux-pca954x.c |   14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c
> b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 550bd36..d3dfbb9 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -189,6 +189,7 @@ static int pca954x_probe(struct i2c_client *client,
>          struct device_node *np = client->dev.of_node;
>          int num, force, class;
>          struct pca954x *data;
> +       u32 fixed_bus_numbers[PCA954X_MAX_NCHANS] = { 0 };
>          int ret;
> 
>          if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
> @@ -214,6 +215,17 @@ static int pca954x_probe(struct i2c_client *client,
>                          if (ret < 0)
>                                  return ret;
>                  }
> +
> +               /* Get fixed bus numbers */
> +               ret = of_property_read_u32_array(np, "fixed-bus-numbers",
> +                       fixed_bus_numbers, chips[id->driver_data].nchans);
> +               if (ret == -EINVAL)
> +                       ret = 0;        /* missing dtb node is not an error
> */ +               if (ret < 0) {
> +                       dev_err(&client->dev, "fixed-bus-numbers: "
> +                       "invalid format\n");
> +                       return ret;
> +               }
>          }
> 
>          /* Write the mux register at addr to verify
> @@ -240,6 +252,8 @@ static int pca954x_probe(struct i2c_client *client,
>                          } else
>                                  /* discard unconfigured channels */
>                                  break;
> +               } else {
> +                       force = fixed_bus_numbers[num];
>                  }
> 
>                  data->virt_adaps[num] =
> 
> > Hi Frank,
> > 
> > On Wednesday 19 March 2014 18:32:50 Frank Bormann wrote:
> >> Hi Everyone,
> >> 
> >> I am looking at the i2c_pca954x i2c bus mux driver in linux-stable. My
> >> goal
> >> is to have the slave buses show up in Linux with static bus numbers.
> >> Ideally, I want to define the first bus number to use in the dtb.
> >> 
> >> It seems, the driver has already some support for static bus numbers as
> >> its
> >> probe function checks for the existence of a struct pca954x_platform_data
> >> instance in client->dev.platform_data and pca954x_platform_mode struct it
> >> points to has a member adap_id that seems to be doing exactly that
> >> judging
> >> by its documentation. However, calls made to pca954x_probe always have to
> >> platform_data pointer being passed in through client set to null.
> >> 
> >> In addition to that, the recent addition to the driver of a reset gpio to
> >> be configured reads directly from the dtb in the probe function.
> >> 
> >> I am unsure about how to set this up properly. On one hand, platform_data
> >> is being passed in to the probe function, which seem to indicate, there
> >> may be some generic place in the i2c core code to set driver-specific
> >> configuration, on the other hand, the recent reset gpio addition to the
> >> driver seems to indicate that the driver's probe function is in fact the
> >> right place to read additional configuration from the dtb.
> >> 
> >> Any help is greatly appreciated.
> > 
> > You're looking at two different configuration mechanisms, which probably
> > explains your confusion.
> > 
> > Platform data is the oldest mechanism used to pass configuration
> > information to the driver. This is largely used through the Linux kernel
> > on a wide range of architectures. The idea is to store device-specific
> > configuration information in board files (as you mentioned DT I'll assume
> > you're working on ARM, so that would be arch/arm/mach-*/board-*.c) using
> > driver-specific structures and associate those structures with devices.
> > Drivers can then retrieve the platform data at probe time and configure
> > the device accordingly.
> > 
> > The way platform data is associated with devices depends on the bus type.
> > For I2C the i2c_board_info structure, used to instantiate I2C devices in
> > board code, has a void *platform_data field that can be set to point to a
> > platform data structure. You can find an example of this in the
> > i2c3_devices array in arch/arm/mach-shmobile/board-kzm9g.c.
> > 
> > Device tree (DT) is a newer mechanism to specify hardware configuration.
> > Instead of relying on C board files that contain a mix a code and data,
> > the
> > platform is described in a tree-like structure of devices with properties
> > associated to all those devices. That structure is called the device tree
> > and is compiled as a binary that gets passed to the kernel at boot time.
> > When using the device tree, drivers don't receive platform data anymore
> > but are responsible for parsing the content of the device tree to read
> > platform- specific hardware configuration information. The content of
> > device tree nodes is defined in documents called DT bindings that can be
> > found in
> > Documentation/devicetree/bindings/ (i2c/i2c-mux-pca954x.txt in this case).
> > 
> > A NULL platform_data pointer can then mean either that your platform boots
> > using the device tree, or that the board file doesn't specify any platform
> > data for the device in case of legacy (non-DT) boot. There's also a hybrid
> > method that can be used to associated platform data declared in C files to
> > DT nodes, but that's for special cases only and shouldn't be used for the
> > pca954x.
> > 
> > This being said, if your platform uses the device tree, you shouldn't (in
> > theory at least) need static I2C bus numbers. This is why there is no DT
> > property similar to the platform data adap_id field defined in the pca954x
> > DT bindings.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be specified in the dts
  2014-04-17 15:46         ` [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be specified in the dts Laurent Pinchart
@ 2014-04-17 18:00           ` Laxman Dewangan
       [not found]             ` <535016B5.7060006-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
       [not found]           ` <53501564.1090607@yahoo.com>
  1 sibling, 1 reply; 13+ messages in thread
From: Laxman Dewangan @ 2014-04-17 18:00 UTC (permalink / raw
  To: Laurent Pinchart, Frank Bormann; +Cc: Rodolfo Giometti, Linux I2C List

On Thursday 17 April 2014 09:16 PM, Laurent Pinchart wrote:
> +       u32 fixed_bus_numbers[PCA954X_MAX_NCHANS] = { 0 };
>           int ret;
>
>           if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
> @@ -214,6 +215,17 @@ static int pca954x_probe(struct i2c_client *client,
>                           if (ret < 0)
>                                   return ret;
>                   }
> +
> +               /* Get fixed bus numbers */
> +               ret = of_property_read_u32_array(np, "fixed-bus-numbers",
> +                       fixed_bus_numbers, chips[id->driver_data].nchans);
> +               if (ret == -EINVAL)
> +                       ret = 0;        /* missing dtb node is not an error
> */ +               if (ret < 0) {
> +                       dev_err(&client->dev, "fixed-bus-numbers: "
> +                       "invalid format\n");
> +                       return ret;
> +               }
>           }
>

Hi Frank,
Why not you use aliases on DTS instead of changing driver?
You can alias the bus number with the child node name. On the following, 
the bus will be registerd as 6, 7, 8 and 9.
i2c-core already support this.

         aliases {
                 i2c6 = &pca9546_i2c0;
                 i2c7 = &pca9546_i2c1;
                 i2c8 = &pca9546_i2c2;
                 i2c9 = &pca9546_i2c3;
         };


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be specified in the dts
       [not found]             ` <53501564.1090607-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
@ 2014-04-17 20:15               ` Laurent Pinchart
       [not found]                 ` <CAE6_GsvJpajY==6MJExo3T7FrVF_LNGcoozq0N5KEBho9y5NWw@mail.gmail.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2014-04-17 20:15 UTC (permalink / raw
  To: Frank Bormann; +Cc: Rodolfo Giometti, Linux I2C List

Hi Frank,

On Thursday 17 April 2014 13:54:44 Frank Bormann wrote:
> Hi Laurent,
> 
> I have a pca9546 on one of my i2c buses. This will create four mux buses in
> Linux. One of those mux buses then has a pca9542 connected to it, creating
> another two mux buses on top of the first ones.
> 
> The 3.8 kernel, I was initially using enumerated, this as:
> 
> pca9546: 3, 4, 5, 6
> pca9542: 7, 8
> 
> However, the new 3.12 kernel, I am using now, has changed that enumeration
> to:
> 
> pca9546: 3, 4, 7, 8
> pca9542: 5, 6
> 
> As you may imagine, the pca9542 is connected to bus 4. Apparently,
> previously, it would finish the initalization of pca9546 before dealing
> with the pca9542. Now it seems to do the pca9542 initialization as soon as
> it sees it on mux bus 4. My application however expects the pca9546 buses
> to be on incremental bus numbers.

I understand that the bus numbers changed, but you still haven't explain *why* 
you need to have fixed bus numbers. Why do you need to know the bus number at 
all ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be specified in the dts
       [not found]                   ` <CAE6_GsvJpajY==6MJExo3T7FrVF_LNGcoozq0N5KEBho9y5NWw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-04-17 20:42                     ` Laurent Pinchart
  0 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-04-17 20:42 UTC (permalink / raw
  To: Frank Bormann; +Cc: Rodolfo Giometti, Linux I2C List

Hi Frank,

On Thursday 17 April 2014 16:26:04 Frank Bormann wrote:
> A user-space application, which is being configured with the first mux bus
> number, executes i2cget/i2cset commands, specifying the i2c bus number and
> expecting them to be incremental from the first number.

I won't venture to comment on whether bus numbering has ever been considered 
to be a kernel ABI, other people should be able to comment on that. Any chance 
to fix the application to find the bus numbers dynamically ?

> On Thu, Apr 17, 2014 at 4:15 PM, Laurent Pinchart wrote:
> > On Thursday 17 April 2014 13:54:44 Frank Bormann wrote:
> > > Hi Laurent,
> > > 
> > > I have a pca9546 on one of my i2c buses. This will create four mux buses
> > > in Linux. One of those mux buses then has a pca9542 connected to it,
> > > creating another two mux buses on top of the first ones.
> > > 
> > > The 3.8 kernel, I was initially using enumerated, this as:
> > > 
> > > pca9546: 3, 4, 5, 6
> > > pca9542: 7, 8
> > > 
> > > However, the new 3.12 kernel, I am using now, has changed that
> > > enumeration to:
> > > 
> > > pca9546: 3, 4, 7, 8
> > > pca9542: 5, 6
> > > 
> > > As you may imagine, the pca9542 is connected to bus 4. Apparently,
> > > previously, it would finish the initalization of pca9546 before dealing
> > > with the pca9542. Now it seems to do the pca9542 initialization as soon
> > > as it sees it on mux bus 4. My application however expects the pca9546
> > > buses to be on incremental bus numbers.
> > 
> > I understand that the bus numbers changed, but you still haven't explain
> > *why* you need to have fixed bus numbers. Why do you need to know the bus
> > number at all ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be specified in the dts
       [not found]             ` <535016B5.7060006-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2014-04-22 21:46               ` Frank Bormann
  0 siblings, 0 replies; 13+ messages in thread
From: Frank Bormann @ 2014-04-22 21:46 UTC (permalink / raw
  To: Laxman Dewangan; +Cc: Laurent Pinchart, Rodolfo Giometti, Linux I2C List

On 17/04/14 02:00 PM, Laxman Dewangan wrote:
> Hi Frank,
> Why not you use aliases on DTS instead of changing driver?
> You can alias the bus number with the child node name. On the following, the bus
> will be registerd as 6, 7, 8 and 9.
> i2c-core already support this.
>
>          aliases {
>                  i2c6 = &pca9546_i2c0;
>                  i2c7 = &pca9546_i2c1;
>                  i2c8 = &pca9546_i2c2;
>                  i2c9 = &pca9546_i2c3;
>          };

Well, this seems to work indeed for newer kernels. I was originally working on 
3.8 that did not yet have this feature. However, this is kind of an hard to find 
feature. I didn't see any references in the i2c device tree bindings 
documentation. So, unless you actually read the i2c core code, it is very easy 
to miss.

Anyway, feel free to disregard my patch.

Frank

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-04-22 21:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-19 22:32 platform_data in i2c device drivers Frank Bormann
     [not found] ` <532A1B12.8080400-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
2014-03-20  0:10   ` Laurent Pinchart
2014-03-20 16:12     ` Frank Bormann
     [not found]       ` <532B1367.8050906-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
2014-03-20 16:25         ` Ben Dooks
     [not found]           ` <532B1689.3080202-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
2014-03-20 17:16             ` Frank Bormann
     [not found]               ` <532B2273.8040004-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
2014-03-20 17:51                 ` Laurent Pinchart
2014-03-21 15:55                   ` Frank Bormann
     [not found]                     ` <532C60EC.3060405-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
2014-03-21 16:01                       ` Laurent Pinchart
     [not found]     ` <534FF708.7040409@yahoo.com>
     [not found]       ` <534FF708.7040409-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
2014-04-17 15:46         ` [PATCH] i2c-mux-pca954x: allow downstream bus numbers to be specified in the dts Laurent Pinchart
2014-04-17 18:00           ` Laxman Dewangan
     [not found]             ` <535016B5.7060006-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-04-22 21:46               ` Frank Bormann
     [not found]           ` <53501564.1090607@yahoo.com>
     [not found]             ` <53501564.1090607-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
2014-04-17 20:15               ` Laurent Pinchart
     [not found]                 ` <CAE6_GsvJpajY==6MJExo3T7FrVF_LNGcoozq0N5KEBho9y5NWw@mail.gmail.com>
     [not found]                   ` <CAE6_GsvJpajY==6MJExo3T7FrVF_LNGcoozq0N5KEBho9y5NWw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-04-17 20:42                     ` Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.