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

[Peter Rosin <peda@lysator.liu.se>]

Hi Guenter,

On 2016-01-04 16:37, Guenter Roeck wrote:
> On 01/04/2016 07:10 AM, Peter Rosin wrote:
>> From: Peter Rosin <peda@axentia.se>
>>
>> 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 <peda@axentia.se>
>> ---
>>   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