[PATCH] extcon: arizona: Get pdata from arizona structure not device

[PATCH] extcon: arizona: Get pdata from arizona structure not device

[Charles Keepax]

In the case of a device tree system there will be no pdata attached to
the device, causing us to deference a NULL pointer. Better to take the
pdata from the Arizona structure as this will always exist and we know
will have been populated since it is populated by the MFD device which
binds in the extcon driver.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index ec9a14e..178454d 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1079,7 +1079,7 @@ static void arizona_micd_set_level(struct arizona *arizona, int index,
 static int arizona_extcon_probe(struct platform_device *pdev)
 {
 	struct arizona *arizona = dev_get_drvdata(pdev->dev.parent);
-	struct arizona_pdata *pdata;
+	struct arizona_pdata *pdata = &arizona->pdata;
 	struct arizona_extcon_info *info;
 	unsigned int val;
 	int jack_irq_fall, jack_irq_rise;
@@ -1088,8 +1088,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	if (!arizona->dapm || !arizona->dapm->card)
 		return -EPROBE_DEFER;
 
-	pdata = dev_get_platdata(arizona->dev);
-
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info) {
 		dev_err(&pdev->dev, "Failed to allocate memory\n");
-- 
1.7.2.5

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Chanwoo Choi]

On 09/28/2013 11:34 PM, Charles Keepax wrote:
> In the case of a device tree system there will be no pdata attached to
> the device, causing us to deference a NULL pointer. Better to take the
> pdata from the Arizona structure as this will always exist and we know
> will have been populated since it is populated by the MFD device which
> binds in the extcon driver.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index ec9a14e..178454d 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -1079,7 +1079,7 @@ static void arizona_micd_set_level(struct arizona *arizona, int index,
>  static int arizona_extcon_probe(struct platform_device *pdev)
>  {
>  	struct arizona *arizona = dev_get_drvdata(pdev->dev.parent);
> -	struct arizona_pdata *pdata;
> +	struct arizona_pdata *pdata = &arizona->pdata;
>  	struct arizona_extcon_info *info;
>  	unsigned int val;
>  	int jack_irq_fall, jack_irq_rise;
> @@ -1088,8 +1088,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>  	if (!arizona->dapm || !arizona->dapm->card)
>  		return -EPROBE_DEFER;
>  
> -	pdata = dev_get_platdata(arizona->dev);
> -
>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>  	if (!info) {
>  		dev_err(&pdev->dev, "Failed to allocate memory\n");
> 

No, extcon-arizona driver don't currently support DT to get platform data.
I cannot find some dt function to parse data from dts file.
You have to implement extcon-arizona driver by using DT binding style
to get platform data. I think this patch is not necessary.

Thanks
Chanwoo Choi

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Charles Keepax]

On Mon, Sep 30, 2013 at 08:37:30AM +0900, Chanwoo Choi wrote:
> No, extcon-arizona driver don't currently support DT to get platform data.
> I cannot find some dt function to parse data from dts file.
> You have to implement extcon-arizona driver by using DT binding style
> to get platform data. I think this patch is not necessary.

Currently the Arizona MFD driver reads the device tree
information and populates the pdata structure, this happens in
drivers/mfd/arizona-core.c. Then the various drivers just use the
pdata as normal.

Admittedly, at the moment we don't parse any data for the extcon
driver but without this patch we will attempt to use a NULL
pointer on device tree systems.

I would also be happy to implement this as a NULL check on the
pdata when we use it if that is preferable? But since we have the
cached pdata seems we might as well use it.

Thanks,
Charles

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Chanwoo Choi]

On 09/30/2013 06:52 PM, Charles Keepax wrote:
> On Mon, Sep 30, 2013 at 08:37:30AM +0900, Chanwoo Choi wrote:
>> No, extcon-arizona driver don't currently support DT to get platform data.
>> I cannot find some dt function to parse data from dts file.
>> You have to implement extcon-arizona driver by using DT binding style
>> to get platform data. I think this patch is not necessary.
> 
> Currently the Arizona MFD driver reads the device tree
> information and populates the pdata structure, this happens in
> drivers/mfd/arizona-core.c. Then the various drivers just use the
> pdata as normal.
> 
> Admittedly, at the moment we don't parse any data for the extcon
> driver but without this patch we will attempt to use a NULL
> pointer on device tree systems.
> 
> I would also be happy to implement this as a NULL check on the
> pdata when we use it if that is preferable? But since we have the
> cached pdata seems we might as well use it.
> 

I find below pdata list for extcon-arizona driver.
But, drivers/mfd/arizona-core.c don't parse dt data for below pdata list
of extcon-arizona. Did you test this patch for extcon-arizona operation?

arizona->pdata.micd_pol_gpio
arizona->pdata.micd_force_micbias	
arizona->pdata.hpdet_id_gpio
arizona->pdata.hpdet_acc_id
arizona->pdata.hpdet_acc_id_line
arizona->pdata.micd_detect_debounce
arizona->pdata.jd_gpio5
arizona->pdata.micd_timeout
arizona->pdata.num_micd_configs
arizona->pdata.micd_configs
arizona->pdata.micd_bias_start_time
arizona->pdata.micd_rate
arizona->pdata.micd_dbtime
arizona->pdata.num_micd_ranges
...

If you fix NULL pointer error about pdata,
I think only that extcon-arizona modify it as following:

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index e557130..3429906 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1088,6 +1088,10 @@ static int arizona_extcon_probe(struct platform_device *pdev)
                return -EPROBE_DEFER;
 
        pdata = dev_get_platdata(arizona->dev);
+       if (!pdata) {
+               dev_err(&pdev->dev, "Failed to get platform data\n");
+               return -EINVAL;
+       }
 
        info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
        if (!info) {

Thanks,
Chanwoo Choi

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Chanwoo Choi]

On 10/01/2013 08:04 AM, Chanwoo Choi wrote:
> On 09/30/2013 06:52 PM, Charles Keepax wrote:
>> On Mon, Sep 30, 2013 at 08:37:30AM +0900, Chanwoo Choi wrote:
>>> No, extcon-arizona driver don't currently support DT to get platform data.
>>> I cannot find some dt function to parse data from dts file.
>>> You have to implement extcon-arizona driver by using DT binding style
>>> to get platform data. I think this patch is not necessary.
>>
>> Currently the Arizona MFD driver reads the device tree
>> information and populates the pdata structure, this happens in
>> drivers/mfd/arizona-core.c. Then the various drivers just use the
>> pdata as normal.
>>
>> Admittedly, at the moment we don't parse any data for the extcon
>> driver but without this patch we will attempt to use a NULL
>> pointer on device tree systems.
>>
>> I would also be happy to implement this as a NULL check on the
>> pdata when we use it if that is preferable? But since we have the
>> cached pdata seems we might as well use it.
>>
> 
> I find below pdata list for extcon-arizona driver.
> But, drivers/mfd/arizona-core.c don't parse dt data for below pdata list
> of extcon-arizona. Did you test this patch for extcon-arizona operation?
> 
> arizona->pdata.micd_pol_gpio
> arizona->pdata.micd_force_micbias	
> arizona->pdata.hpdet_id_gpio
> arizona->pdata.hpdet_acc_id
> arizona->pdata.hpdet_acc_id_line
> arizona->pdata.micd_detect_debounce
> arizona->pdata.jd_gpio5
> arizona->pdata.micd_timeout
> arizona->pdata.num_micd_configs
> arizona->pdata.micd_configs
> arizona->pdata.micd_bias_start_time
> arizona->pdata.micd_rate
> arizona->pdata.micd_dbtime
> arizona->pdata.num_micd_ranges
> ...
> 
> If you fix NULL pointer error about pdata,
> I think only that extcon-arizona modify it as following:
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index e557130..3429906 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -1088,6 +1088,10 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>                 return -EPROBE_DEFER;
>  
>         pdata = dev_get_platdata(arizona->dev);
> +       if (!pdata) {
> +               dev_err(&pdev->dev, "Failed to get platform data\n");
> +               return -EINVAL;
> +       }
>  
>         info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>         if (!info) {
> 

Additionally,I have a question.
As I mentioned, extcon-arizon driver don't get pdata from dt parsing.
I think extcon-arizona haven't operated without pdata.

Did you only build test for extcon-arizona?

I'd like you to implement parse function for extcon-arizona
to solve this issue(NULL pointer error).

Thanks
Chanwoo Choi

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

On Tue, Oct 01, 2013 at 08:14:39AM +0900, Chanwoo Choi wrote:

> Additionally,I have a question.
> As I mentioned, extcon-arizon driver don't get pdata from dt parsing.
> I think extcon-arizona haven't operated without pdata.

It should work fine on Wolfson reference systems and anything cloned
from them (it did when I was working there, that was the intened goal).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

On Tue, Oct 01, 2013 at 08:04:09AM +0900, Chanwoo Choi wrote:
> On 09/30/2013 06:52 PM, Charles Keepax wrote:

> > I would also be happy to implement this as a NULL check on the
> > pdata when we use it if that is preferable? But since we have the
> > cached pdata seems we might as well use it.

> I find below pdata list for extcon-arizona driver.
> But, drivers/mfd/arizona-core.c don't parse dt data for below pdata list
> of extcon-arizona. Did you test this patch for extcon-arizona operation?

Charles had some patches out for review but there were issues with the
DT binding that meant it needs a respin - it will need to support extcon
at some point.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Charles Keepax]

On Tue, Oct 01, 2013 at 08:04:09AM +0900, Chanwoo Choi wrote:
> On 09/30/2013 06:52 PM, Charles Keepax wrote:
> > On Mon, Sep 30, 2013 at 08:37:30AM +0900, Chanwoo Choi wrote:
> >> No, extcon-arizona driver don't currently support DT to get platform data.
> >> I cannot find some dt function to parse data from dts file.
> >> You have to implement extcon-arizona driver by using DT binding style
> >> to get platform data. I think this patch is not necessary.
> > 
> > Currently the Arizona MFD driver reads the device tree
> > information and populates the pdata structure, this happens in
> > drivers/mfd/arizona-core.c. Then the various drivers just use the
> > pdata as normal.
> > 
> > Admittedly, at the moment we don't parse any data for the extcon
> > driver but without this patch we will attempt to use a NULL
> > pointer on device tree systems.
> > 
> > I would also be happy to implement this as a NULL check on the
> > pdata when we use it if that is preferable? But since we have the
> > cached pdata seems we might as well use it.
> > 
> 
> I find below pdata list for extcon-arizona driver.
> But, drivers/mfd/arizona-core.c don't parse dt data for below pdata list
> of extcon-arizona. Did you test this patch for extcon-arizona operation?

The extcon driver will function using its default settings if
blank pdata is provided, so it has been tested with those
settings. I am presently working on device tree bindings for the
pdata for the extcon driver which I hope to send a new spin of
upstream this week or next, so it has been tested against the
first version of those patches as well.

> +       if (!pdata) {
> +               dev_err(&pdev->dev, "Failed to get platform data\n");
> +               return -EINVAL;
> +       }

I think this would be unecessary churn as I would have to take it
out and replace it with this patch once the bindings are complete.

That said I don't really mind waiting to merge this patch until I
have upstreamed the first of the device tree bindings for the
extcon driver if you feel strongly about it? Or indeed I could
even send the patch as part of that series?

Thanks,
Charles

Re: [PATCH] extcon: arizona: Get pdata from arizona structure not device

[Chanwoo Choi]

On 10/01/2013 05:22 PM, Charles Keepax wrote:
> On Tue, Oct 01, 2013 at 08:04:09AM +0900, Chanwoo Choi wrote:
>> On 09/30/2013 06:52 PM, Charles Keepax wrote:
>>> On Mon, Sep 30, 2013 at 08:37:30AM +0900, Chanwoo Choi wrote:
>>>> No, extcon-arizona driver don't currently support DT to get platform data.
>>>> I cannot find some dt function to parse data from dts file.
>>>> You have to implement extcon-arizona driver by using DT binding style
>>>> to get platform data. I think this patch is not necessary.
>>>
>>> Currently the Arizona MFD driver reads the device tree
>>> information and populates the pdata structure, this happens in
>>> drivers/mfd/arizona-core.c. Then the various drivers just use the
>>> pdata as normal.
>>>
>>> Admittedly, at the moment we don't parse any data for the extcon
>>> driver but without this patch we will attempt to use a NULL
>>> pointer on device tree systems.
>>>
>>> I would also be happy to implement this as a NULL check on the
>>> pdata when we use it if that is preferable? But since we have the
>>> cached pdata seems we might as well use it.
>>>
>>
>> I find below pdata list for extcon-arizona driver.
>> But, drivers/mfd/arizona-core.c don't parse dt data for below pdata list
>> of extcon-arizona. Did you test this patch for extcon-arizona operation?
> 
> The extcon driver will function using its default settings if
> blank pdata is provided, so it has been tested with those
> settings. I am presently working on device tree bindings for the
> pdata for the extcon driver which I hope to send a new spin of
> upstream this week or next, so it has been tested against the
> first version of those patches as well.

OK,
I check arizona->pdata including the pdata field for extcon-arizona.

Applied it.

Thanks,
Chanwoo Choi