Re: [PATCH 01/10] i2c-mux: add common core data for every mux instance

[Peter Rosin]

Hi Guenter,

On 2016-01-04 16:37, Guenter Roeck wrote:
> On 01/04/2016 07:10 AM, Peter Rosin wrote:
>> From: Peter Rosin 
>>
>> The initial core mux structure starts off small with only the parent
>> adapter pointer, which all muxes have, and a priv pointer for mux
>> driver private data.
>>
>> Add i2c_mux_alloc function to unify the creation of a mux.
>>
>> Where appropriate, pass around the mux core structure instead of the
>> parent adapter or the driver private data.
>>
>> Remove the parent adapter pointer from the driver private data for all
>> mux drivers.
>>
>> Signed-off-by: Peter Rosin 
>> ---
>>   drivers/i2c/i2c-mux.c  | 35 
>> -
>>   drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 24 +++-
>>   drivers/i2c/muxes/i2c-mux-gpio.c   | 20 +
>>   drivers/i2c/muxes/i2c-mux-pca9541.c| 36 
>> --
>>   drivers/i2c/muxes/i2c-mux-pca954x.c| 22 +-
>>   drivers/i2c/muxes/i2c-mux-pinctrl.c| 24 +++-
>>   drivers/i2c/muxes/i2c-mux-reg.c| 25 -
>>   include/linux/i2c-mux.h| 14 +++-
>>   8 files changed, 129 insertions(+), 71 deletions(-)
>>

*snip*

>> +struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
>> +{
>> +struct i2c_mux_core *muxc;
>> +
>> +muxc = devm_kzalloc(dev, sizeof(*muxc), GFP_KERNEL);
>> +if (!muxc)
>> +return NULL;
>> +if (sizeof_priv) {
>> +muxc->priv = devm_kzalloc(dev, sizeof_priv, GFP_KERNEL);
>> +if (!muxc->priv)
>> +goto fail;
>> +}
> 
> Why not just allocate sizeof(*muxc) + sizeof_priv in a single operation
> and then assign muxc->priv to muxc + 1 if sizeof_priv > 0 ?

Why indeed, good suggestion.

*snip*

>> @@ -134,13 +134,14 @@ static int i2c_arbitrator_probe(struct platform_device 
>> *pdev)
>>   return -EINVAL;
>>   }
>>
>> -arb = devm_kzalloc(dev, sizeof(*arb), GFP_KERNEL);
>> -if (!arb) {
>> -dev_err(dev, "Cannot allocate i2c_arbitrator_data\n");
>> +muxc = i2c_mux_alloc(dev, sizeof(*arb));
>> +if (!muxc) {
>> +dev_err(dev, "Cannot allocate i2c_mux_core structure\n");
> 
> Unnecessary error message.
> 

Right, I'll remove that (and the others just like it).

I'll see if I can cook up a v2 that also converts the i2c muxes elsewhere in
drivers/ that I wasn't aware of.

Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/10] i2c-mux: add common core data for every mux instance

[kbuild test robot]

Hi Peter,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.4-rc8 next-20160104]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Peter-Rosin/i2c-mux-cleanup-and-locking-update/20160104-231355
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
config: i386-randconfig-i0-01042049 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/iio/imu/inv_mpu6050/inv_mpu_core.c: In function 'inv_mpu_probe':
>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c:845:40: warning: passing argument 
>> 1 of 'i2c_add_mux_adapter' from incompatible pointer type 
>> [-Wincompatible-pointer-types]
 st->mux_adapter = i2c_add_mux_adapter(client->adapter,
   ^
   In file included from drivers/iio/imu/inv_mpu6050/inv_mpu_core.c:26:0:
   include/linux/i2c-mux.h:48:21: note: expected 'struct i2c_mux_core *' but 
argument is of type 'struct i2c_adapter *'
struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
^
--
   drivers/of/unittest.c: In function 'unittest_i2c_mux_probe':
>> drivers/of/unittest.c:1730:38: warning: passing argument 1 of 
>> 'i2c_add_mux_adapter' from incompatible pointer type 
>> [-Wincompatible-pointer-types]
  stm->adap[i] = i2c_add_mux_adapter(adap, dev, client,
 ^
   In file included from drivers/of/unittest.c:24:0:
   include/linux/i2c-mux.h:48:21: note: expected 'struct i2c_mux_core *' but 
argument is of type 'struct i2c_adapter *'
struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
^

vim +/i2c_add_mux_adapter +845 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c

452204ae Sachin Kamat2013-07-30  829return result;
09a642b7 Ge Gao  2013-02-02  830}
09a642b7 Ge Gao  2013-02-02  831result = 
inv_mpu6050_probe_trigger(indio_dev);
09a642b7 Ge Gao  2013-02-02  832if (result) {
09a642b7 Ge Gao  2013-02-02  833
dev_err(>client->dev, "trigger probe fail %d\n", result);
09a642b7 Ge Gao  2013-02-02  834goto out_unreg_ring;
09a642b7 Ge Gao  2013-02-02  835}
09a642b7 Ge Gao  2013-02-02  836  
09a642b7 Ge Gao  2013-02-02  837INIT_KFIFO(st->timestamps);
09a642b7 Ge Gao  2013-02-02  838
spin_lock_init(>time_stamp_lock);
09a642b7 Ge Gao  2013-02-02  839result = 
iio_device_register(indio_dev);
09a642b7 Ge Gao  2013-02-02  840if (result) {
09a642b7 Ge Gao  2013-02-02  841
dev_err(>client->dev, "IIO register fail %d\n", result);
09a642b7 Ge Gao  2013-02-02  842goto out_remove_trigger;
09a642b7 Ge Gao  2013-02-02  843}
09a642b7 Ge Gao  2013-02-02  844  
3a2ecc3d Srinivas Pandruvada 2014-12-05 @845st->mux_adapter = 
i2c_add_mux_adapter(client->adapter,
3a2ecc3d Srinivas Pandruvada 2014-12-05  846
  >dev,
3a2ecc3d Srinivas Pandruvada 2014-12-05  847
  indio_dev,
3a2ecc3d Srinivas Pandruvada 2014-12-05  848
  0, 0, 0,
3a2ecc3d Srinivas Pandruvada 2014-12-05  849
  inv_mpu6050_select_bypass,
3a2ecc3d Srinivas Pandruvada 2014-12-05  850
  inv_mpu6050_deselect_bypass);
3a2ecc3d Srinivas Pandruvada 2014-12-05  851if (!st->mux_adapter) {
3a2ecc3d Srinivas Pandruvada 2014-12-05  852result = -ENODEV;
3a2ecc3d Srinivas Pandruvada 2014-12-05  853goto out_unreg_device;

:: The code at line 845 was first introduced by commit
:: 3a2ecc3d2dce6e051b6afc319bb380c829e4e4fd iio: imu: inv_mpu6050: Add i2c 
mux for by pass

:: TO: Srinivas Pandruvada 
:: CC: Jonathan Cameron 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH 01/10] i2c-mux: add common core data for every mux instance

[Guenter Roeck]

On 01/04/2016 07:10 AM, Peter Rosin wrote:

From: Peter Rosin 

The initial core mux structure starts off small with only the parent
adapter pointer, which all muxes have, and a priv pointer for mux
driver private data.

Add i2c_mux_alloc function to unify the creation of a mux.

Where appropriate, pass around the mux core structure instead of the
parent adapter or the driver private data.

Remove the parent adapter pointer from the driver private data for all
mux drivers.

Signed-off-by: Peter Rosin 
---
  drivers/i2c/i2c-mux.c  | 35 -
  drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 24 +++-
  drivers/i2c/muxes/i2c-mux-gpio.c   | 20 +
  drivers/i2c/muxes/i2c-mux-pca9541.c| 36 --
  drivers/i2c/muxes/i2c-mux-pca954x.c| 22 +-
  drivers/i2c/muxes/i2c-mux-pinctrl.c| 24 +++-
  drivers/i2c/muxes/i2c-mux-reg.c| 25 -
  include/linux/i2c-mux.h| 14 +++-
  8 files changed, 129 insertions(+), 71 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 00fc5b1c7b66..99fd9106abc6 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -31,8 +31,8 @@
  struct i2c_mux_priv {
struct i2c_adapter adap;
struct i2c_algorithm algo;
+   struct i2c_mux_core *muxc;

-   struct i2c_adapter *parent;
struct device *mux_dev;
void *mux_priv;
u32 chan_id;
@@ -45,7 +45,8 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
   struct i2c_msg msgs[], int num)
  {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_mux_core *muxc = priv->muxc;
+   struct i2c_adapter *parent = muxc->parent;
int ret;

/* Switch to the right mux port and perform the transfer. */
@@ -65,7 +66,8 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
  int size, union i2c_smbus_data *data)
  {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_mux_core *muxc = priv->muxc;
+   struct i2c_adapter *parent = muxc->parent;
int ret;

/* Select the right mux port and perform the transfer. */
@@ -84,7 +86,7 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
  static u32 i2c_mux_functionality(struct i2c_adapter *adap)
  {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_adapter *parent = priv->muxc->parent;

return parent->algo->functionality(parent);
  }
@@ -102,7 +104,27 @@ static unsigned int i2c_mux_parent_classes(struct 
i2c_adapter *parent)
return class;
  }

-struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
+{
+   struct i2c_mux_core *muxc;
+
+   muxc = devm_kzalloc(dev, sizeof(*muxc), GFP_KERNEL);
+   if (!muxc)
+   return NULL;
+   if (sizeof_priv) {
+   muxc->priv = devm_kzalloc(dev, sizeof_priv, GFP_KERNEL);
+   if (!muxc->priv)
+   goto fail;
+   }


Why not just allocate sizeof(*muxc) + sizeof_priv in a single operation
and then assign muxc->priv to muxc + 1 if sizeof_priv > 0 ?


+   return muxc;
+
+fail:
+   devm_kfree(dev, muxc);
+   return NULL;
+}
+EXPORT_SYMBOL_GPL(i2c_mux_alloc);
+
+struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
struct device *mux_dev,
void *mux_priv, u32 force_nr, u32 chan_id,
unsigned int class,
@@ -111,6 +133,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter 
*parent,
int (*deselect) (struct i2c_adapter *,
 void *, u32))
  {
+   struct i2c_adapter *parent = muxc->parent;
struct i2c_mux_priv *priv;
char symlink_name[20];
int ret;
@@ -120,7 +143,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter 
*parent,
return NULL;

/* Set up private adapter data */
-   priv->parent = parent;
+   priv->muxc = muxc;
priv->mux_dev = mux_dev;
priv->mux_priv = mux_priv;
priv->chan_id = chan_id;
diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c 
b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index 402e3a6c671a..dd616c0280ad 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -42,7 +42,6 @@
   */

  struct i2c_arbitrator_data {
-   struct i2c_adapter *parent;
struct i2c_adapter *child;
int our_gpio;
int