[PATCH v2 1/2] mfd: sec-core: Remove duplicated device type from sec_pmic_dev

[PATCH v2 1/2] mfd: sec-core: Remove duplicated device type from sec_pmic_dev

[Krzysztof Kozlowski]

The device type was stored in sec_pmic_dev state container twice:
 - unsigned long type (initialized from of_device_id or i2c_device_id)
 - int device_type (initialized as above or from board files when there
   is no DTS)

The 'type' field was never used outside of probe so it can be safely
removed.

Change also the device_type in sec_pmic_dev and sec_platform_data to
unsigned long to avoid any casts.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

---

Changes since v1:
 - Change device_type field of sec_platform_data from int to unsigned long.
 - Update commit msg.
---
 drivers/mfd/sec-core.c           | 5 +++--
 drivers/mfd/sec-irq.c            | 2 +-
 include/linux/mfd/samsung/core.h | 3 +--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index d4682c6cbff5..09fd256abcf6 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);
 
 	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 = device_type;
 	}
 	if (pdata) {
 		sec_pmic->device_type = pdata->device_type;
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index 64e7913aadc6..654e2c1dbf7a 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -385,7 +385,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 				  &sec_pmic->irq_data);
 		break;
 	default:
-		dev_err(sec_pmic->dev, "Unknown device type %d\n",
+		dev_err(sec_pmic->dev, "Unknown device type %lu\n",
 			sec_pmic->device_type);
 		return -EINVAL;
 	}
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 84aaf6c25794..e7e12ebacb9b 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -43,13 +43,12 @@ struct sec_pmic_dev {
 	struct regmap *regmap_pmic;
 	struct i2c_client *i2c;
 
-	int device_type;
+	unsigned long device_type;
 	int irq_base;
 	int irq;
 	struct regmap_irq_chip_data *irq_data;
 
 	int ono;
-	unsigned long type;
 	bool wakeup;
 	bool wtsr_smpl;
 };
-- 
1.9.1

[PATCH v2 2/2] mfd: sec-core: Update sec_pmic documentation

[Krzysztof Kozlowski]

Update the documentation for sec_pmic state container structure to
reflect current code.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

---

Changes since v1:
 - Update documentation after review from Lee Jones.
---
 include/linux/mfd/samsung/core.h | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index e7e12ebacb9b..47d84242940b 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -24,18 +24,23 @@ enum sec_device_type {
 };
 
 /**
- * struct sec_pmic_dev - s5m87xx master device for sub-drivers
- * @dev: master device of the chip (can be used to access platform data)
- * @pdata: pointer to private data used to pass platform data to child
- * @i2c: i2c client private data for regulator
- * @iolock: mutex for serializing io access
- * @irqlock: mutex for buslock
- * @irq_base: base IRQ number for sec-pmic, required for IRQs
- * @irq: generic IRQ number for s5m87xx
- * @ono: power onoff IRQ number for s5m87xx
- * @irq_masks_cur: currently active value
- * @irq_masks_cache: cached hardware value
- * @type: indicate which s5m87xx "variant" is used
+ * struct sec_pmic_dev - s2m/s5m master device for sub-drivers
+ * @dev:		Master device of the chip
+ * @pdata:		Platform data populated with data from DTS
+ *			or board files
+ * @regmap_pmic:	Regmap associated with PMIC's I2C address
+ * @i2c:		I2C client of the main driver
+ * @device_type:	Type of device, matches enum sec_device_type
+ * @irq_base:		Base IRQ number for device, required for IRQs
+ * @irq:		Generic IRQ number for device
+ * @irq_data:		Runtime data structure for IRQ controller
+ * @ono:		Power onoff IRQ number for s5m87xx
+ * @wakeup:		Whether or not this is a wakeup device
+ * @wtsr_smpl:		Whether or not to enable in RTC driver the Watchdog
+ *			Timer Software Reset (registers set to default value
+ *			after PWRHOLD falling) and Sudden Momentary Power Loss
+ *			(PMIC will enter power on sequence after short drop in
+ *			VBATT voltage).
  */
 struct sec_pmic_dev {
 	struct device *dev;
-- 
1.9.1

Re: [PATCH v2 2/2] mfd: sec-core: Update sec_pmic documentation

[Lee Jones]

> Update the documentation for sec_pmic state container structure to
> reflect current code.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> ---
> 
> Changes since v1:
>  - Update documentation after review from Lee Jones.
> ---
>  include/linux/mfd/samsung/core.h | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 2/2] mfd: sec-core: Update sec_pmic documentation

[Lee Jones]

On Tue, 22 Apr 2014, Lee Jones wrote:

> > Update the documentation for sec_pmic state container structure to
> > reflect current code.
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > 
> > ---
> > 
> > Changes since v1:
> >  - Update documentation after review from Lee Jones.
> > ---
> >  include/linux/mfd/samsung/core.h | 29 +++++++++++++++++------------
> >  1 file changed, 17 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> 
> Applied, thanks.

Same here, doesn't apply. Please rebase and resend.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/2] mfd: sec-core: Remove duplicated device type from sec_pmic_dev

[Lee Jones]

> The device type was stored in sec_pmic_dev state container twice:
>  - unsigned long type (initialized from of_device_id or i2c_device_id)
>  - int device_type (initialized as above or from board files when there
>    is no DTS)
> 
> The 'type' field was never used outside of probe so it can be safely
> removed.
> 
> Change also the device_type in sec_pmic_dev and sec_platform_data to
> unsigned long to avoid any casts.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> ---
> 
> Changes since v1:
>  - Change device_type field of sec_platform_data from int to unsigned long.
>  - Update commit msg.
> ---
>  drivers/mfd/sec-core.c           | 5 +++--
>  drivers/mfd/sec-irq.c            | 2 +-
>  include/linux/mfd/samsung/core.h | 3 +--
>  3 files changed, 5 insertions(+), 5 deletions(-)

Patch looks good now. Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/2] mfd: sec-core: Remove duplicated device type from sec_pmic_dev

[Lee Jones]

On Tue, 22 Apr 2014, Krzysztof Kozlowski wrote:

> The device type was stored in sec_pmic_dev state container twice:
>  - unsigned long type (initialized from of_device_id or i2c_device_id)
>  - int device_type (initialized as above or from board files when there
>    is no DTS)
> 
> The 'type' field was never used outside of probe so it can be safely
> removed.
> 
> Change also the device_type in sec_pmic_dev and sec_platform_data to
> unsigned long to avoid any casts.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> ---
> 
> Changes since v1:
>  - Change device_type field of sec_platform_data from int to unsigned long.
>  - Update commit msg.
> ---
>  drivers/mfd/sec-core.c           | 5 +++--
>  drivers/mfd/sec-irq.c            | 2 +-
>  include/linux/mfd/samsung/core.h | 3 +--
>  3 files changed, 5 insertions(+), 5 deletions(-)

What upstream commit are these patches based on? They don't seem to
apply to v3.15-rc2. Could you rebase and resend with my Ack applied
please?

> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index d4682c6cbff5..09fd256abcf6 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);
>  
>  	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 = device_type;
>  	}
>  	if (pdata) {
>  		sec_pmic->device_type = pdata->device_type;
> diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> index 64e7913aadc6..654e2c1dbf7a 100644
> --- a/drivers/mfd/sec-irq.c
> +++ b/drivers/mfd/sec-irq.c
> @@ -385,7 +385,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
>  				  &sec_pmic->irq_data);
>  		break;
>  	default:
> -		dev_err(sec_pmic->dev, "Unknown device type %d\n",
> +		dev_err(sec_pmic->dev, "Unknown device type %lu\n",
>  			sec_pmic->device_type);
>  		return -EINVAL;
>  	}
> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> index 84aaf6c25794..e7e12ebacb9b 100644
> --- a/include/linux/mfd/samsung/core.h
> +++ b/include/linux/mfd/samsung/core.h
> @@ -43,13 +43,12 @@ struct sec_pmic_dev {
>  	struct regmap *regmap_pmic;
>  	struct i2c_client *i2c;
>  
> -	int device_type;
> +	unsigned long device_type;
>  	int irq_base;
>  	int irq;
>  	struct regmap_irq_chip_data *irq_data;
>  
>  	int ono;
> -	unsigned long type;
>  	bool wakeup;
>  	bool wtsr_smpl;
>  };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/2] mfd: sec-core: Remove duplicated device type from sec_pmic_dev

[Krzysztof Kozlowski]

On śro, 2014-04-23 at 14:51 +0100, Lee Jones wrote:
> On Tue, 22 Apr 2014, Krzysztof Kozlowski wrote:
> 
> > The device type was stored in sec_pmic_dev state container twice:
> >  - unsigned long type (initialized from of_device_id or i2c_device_id)
> >  - int device_type (initialized as above or from board files when there
> >    is no DTS)
> > 
> > The 'type' field was never used outside of probe so it can be safely
> > removed.
> > 
> > Change also the device_type in sec_pmic_dev and sec_platform_data to
> > unsigned long to avoid any casts.
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > 
> > ---
> > 
> > Changes since v1:
> >  - Change device_type field of sec_platform_data from int to unsigned long.
> >  - Update commit msg.
> > ---
> >  drivers/mfd/sec-core.c           | 5 +++--
> >  drivers/mfd/sec-irq.c            | 2 +-
> >  include/linux/mfd/samsung/core.h | 3 +--
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> What upstream commit are these patches based on? They don't seem to
> apply to v3.15-rc2. Could you rebase and resend with my Ack applied
> please?
> 

They depend on this:
mfd/rtc: s5m: Do not allocate RTC I2C dummy and regmap for unsupported chipset
https://lkml.org/lkml/2014/4/14/50

Which I thought you applied with Alessandro ack. However this was picked
up also by Andrew Morton:
http://git.cmpxchg.org/cgit/linux-mmots.git/log/

I will rebase them but still this would produce a (rather simple) merge
conflict later.

Best regards,
Krzysztof

> > diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> > index d4682c6cbff5..09fd256abcf6 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);
> >  
> >  	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 = device_type;
> >  	}
> >  	if (pdata) {
> >  		sec_pmic->device_type = pdata->device_type;
> > diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> > index 64e7913aadc6..654e2c1dbf7a 100644
> > --- a/drivers/mfd/sec-irq.c
> > +++ b/drivers/mfd/sec-irq.c
> > @@ -385,7 +385,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> >  				  &sec_pmic->irq_data);
> >  		break;
> >  	default:
> > -		dev_err(sec_pmic->dev, "Unknown device type %d\n",
> > +		dev_err(sec_pmic->dev, "Unknown device type %lu\n",
> >  			sec_pmic->device_type);
> >  		return -EINVAL;
> >  	}
> > diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> > index 84aaf6c25794..e7e12ebacb9b 100644
> > --- a/include/linux/mfd/samsung/core.h
> > +++ b/include/linux/mfd/samsung/core.h
> > @@ -43,13 +43,12 @@ struct sec_pmic_dev {
> >  	struct regmap *regmap_pmic;
> >  	struct i2c_client *i2c;
> >  
> > -	int device_type;
> > +	unsigned long device_type;
> >  	int irq_base;
> >  	int irq;
> >  	struct regmap_irq_chip_data *irq_data;
> >  
> >  	int ono;
> > -	unsigned long type;
> >  	bool wakeup;
> >  	bool wtsr_smpl;
> >  };
> 

Re: [PATCH v2 1/2] mfd: sec-core: Remove duplicated device type from sec_pmic_dev

[Lee Jones]

On Wed, 23 Apr 2014, Krzysztof Kozlowski wrote:

> On śro, 2014-04-23 at 14:51 +0100, Lee Jones wrote:
> > On Tue, 22 Apr 2014, Krzysztof Kozlowski wrote:
> > 
> > > The device type was stored in sec_pmic_dev state container twice:
> > >  - unsigned long type (initialized from of_device_id or i2c_device_id)
> > >  - int device_type (initialized as above or from board files when there
> > >    is no DTS)
> > > 
> > > The 'type' field was never used outside of probe so it can be safely
> > > removed.
> > > 
> > > Change also the device_type in sec_pmic_dev and sec_platform_data to
> > > unsigned long to avoid any casts.
> > > 
> > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > > 
> > > ---
> > > 
> > > Changes since v1:
> > >  - Change device_type field of sec_platform_data from int to unsigned long.
> > >  - Update commit msg.
> > > ---
> > >  drivers/mfd/sec-core.c           | 5 +++--
> > >  drivers/mfd/sec-irq.c            | 2 +-
> > >  include/linux/mfd/samsung/core.h | 3 +--
> > >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > What upstream commit are these patches based on? They don't seem to
> > apply to v3.15-rc2. Could you rebase and resend with my Ack applied
> > please?
> > 
> 
> They depend on this:
> mfd/rtc: s5m: Do not allocate RTC I2C dummy and regmap for unsupported chipset
> https://lkml.org/lkml/2014/4/14/50

Ah, I only just applied this.

Can you rebase on my tree and resubmit please?

> Which I thought you applied with Alessandro ack. However this was picked
> up also by Andrew Morton:
> http://git.cmpxchg.org/cgit/linux-mmots.git/log/

Don't worry about this. Andrew has a script to make them vanish if
applied elsewhere. 

> I will rebase them but still this would produce a (rather simple) merge
> conflict later.
> 
> Best regards,
> Krzysztof
> 
> > > diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> > > index d4682c6cbff5..09fd256abcf6 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);
> > >  
> > >  	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 = device_type;
> > >  	}
> > >  	if (pdata) {
> > >  		sec_pmic->device_type = pdata->device_type;
> > > diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> > > index 64e7913aadc6..654e2c1dbf7a 100644
> > > --- a/drivers/mfd/sec-irq.c
> > > +++ b/drivers/mfd/sec-irq.c
> > > @@ -385,7 +385,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> > >  				  &sec_pmic->irq_data);
> > >  		break;
> > >  	default:
> > > -		dev_err(sec_pmic->dev, "Unknown device type %d\n",
> > > +		dev_err(sec_pmic->dev, "Unknown device type %lu\n",
> > >  			sec_pmic->device_type);
> > >  		return -EINVAL;
> > >  	}
> > > diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> > > index 84aaf6c25794..e7e12ebacb9b 100644
> > > --- a/include/linux/mfd/samsung/core.h
> > > +++ b/include/linux/mfd/samsung/core.h
> > > @@ -43,13 +43,12 @@ struct sec_pmic_dev {
> > >  	struct regmap *regmap_pmic;
> > >  	struct i2c_client *i2c;
> > >  
> > > -	int device_type;
> > > +	unsigned long device_type;
> > >  	int irq_base;
> > >  	int irq;
> > >  	struct regmap_irq_chip_data *irq_data;
> > >  
> > >  	int ono;
> > > -	unsigned long type;
> > >  	bool wakeup;
> > >  	bool wtsr_smpl;
> > >  };
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog