On czw, 2014-04-17 at 09:26 +0100, Lee Jones wrote: > > The device type was stored in sec_pmic state container twice: > > - unsigned long type > > - int device_type > > Can you explain that one of them is in the main container and the > other is stored in pdata, which in turn is located in the main > container, as this implies that both of them are located in the same > place, which is not quite true.
Sure, I'll expand this section. However they were both stored in the same place - struct sec_pmic. Additionally device_type is stored in pdata. > > > The 'type' field was never used outside of probe so it can be safely > > removed. > > > > Signed-off-by: Krzysztof Kozlowski <[email protected]> > > --- > > drivers/mfd/sec-core.c | 5 +++-- > > include/linux/mfd/samsung/core.h | 1 - > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c > > index d4682c6cbff5..fccfd71f8abd 100644 > > --- a/drivers/mfd/sec-core.c > > +++ b/drivers/mfd/sec-core.c > > @@ -251,6 +251,7 @@ static int sec_pmic_probe(struct i2c_client *i2c, > > struct sec_platform_data *pdata = dev_get_platdata(&i2c->dev); > > const struct regmap_config *regmap; > > struct sec_pmic_dev *sec_pmic; > > + unsigned long device_type; > > int ret; > > > > sec_pmic = devm_kzalloc(&i2c->dev, sizeof(struct sec_pmic_dev), > > @@ -262,7 +263,7 @@ static int sec_pmic_probe(struct i2c_client *i2c, > > sec_pmic->dev = &i2c->dev; > > sec_pmic->i2c = i2c; > > sec_pmic->irq = i2c->irq; > > - sec_pmic->type = sec_i2c_get_driver_data(i2c, id); > > + device_type = sec_i2c_get_driver_data(i2c, id); > > Better to change the return type of 'sec_i2c_get_driver_data()' than > to rely on a cast later on. Hmmm... it was like that before Pankaj Dubey change (8f695de515b9). Solving that issue (-Wpointer-to-int-cast) along with keeping 'int' as return type of sec_i2c_get_driver_data would lead to one of: 1. Using temporary variable in ec_i2c_get_driver_data just for cast. static inline int sec_i2c_get_driver_data(struct i2c_client *i2c, const struct i2c_device_id *id) { unsigned long type; if (i2c->dev.of_node) { const struct of_device_id *match; match = of_match_node(sec_dt_match, i2c->dev.of_node); type = (unsigned long)match->data; } type = id->driver_data; return (int)type; } or 2. Double cast: (int)(unsigned long)match->data; For me both are kind a ugly... What do you think? > > > if (sec_pmic->dev->of_node) { > > pdata = sec_pmic_i2c_parse_dt_pdata(sec_pmic->dev); > > @@ -270,7 +271,7 @@ static int sec_pmic_probe(struct i2c_client *i2c, > > ret = PTR_ERR(pdata); > > return ret; > > } > > - pdata->device_type = sec_pmic->type; > > + pdata->device_type = (int)device_type; > > } > > if (pdata) { > > sec_pmic->device_type = pdata->device_type; > > diff --git a/include/linux/mfd/samsung/core.h > > b/include/linux/mfd/samsung/core.h > > index 84aaf6c25794..2633083262ea 100644 > > --- a/include/linux/mfd/samsung/core.h > > +++ b/include/linux/mfd/samsung/core.h > > @@ -49,7 +49,6 @@ struct sec_pmic_dev { > > struct regmap_irq_chip_data *irq_data; > > > > int ono; > > - unsigned long type; > > bool wakeup; > > bool wtsr_smpl; > > }; > > -- > > 1.8.3.2 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
