platform_data in i2c device drivers

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.

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.

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.

Hi Ben,

Quoted 3 lines

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.

Hi Frank,

Quoted 6 lines · On Thursday 20 March 2014 13:16:35 Frank Bormann wrote:

Hi Ben,

Quoted 3 lines

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.

Quoted 8 lines

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.

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?

dev.platform_data will be NULL).

Quoted 21 lines · Laurent Pinchart

Quoted 6 lines · On Thursday 20 March 2014 13:16:35 Frank Bormann wrote:

Hi Ben,

Quoted 3 lines

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.

Quoted 8 lines

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.

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.

From dc4968005ad7211867ac344df958d7e0605910dd Mon Sep 17 00:00:00 2001
From: Frank Bormann <fbormann-/[email protected]>
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-/[email protected]>

---
  drivers/i2c/muxes/i2c-mux-pca954x.c |   14 ++++++++++++++
  1 file changed, 14 insertions(+)

drivers/i2c/muxes/i2c-mux-pca954x.c +13 -0

index 550bd36..d3dfbb9 100644
@@ -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] =

Quoted 74 lines

Hi Frank,

Quoted 19 lines · 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.

Quoted 8 lines

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.

(quoted diff) +4 -0

+       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",

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;
         };

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.

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.

Quoted 27 lines · On Thursday 17 April 2014 13:54:44 Frank Bormann wrote:

Quoted 14 lines

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:

Quoted 8 lines

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 ?