[PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Andy Shevchenko]

The commit 942c7924a51e introduced a check for ACPI handle for the
device that never appears on any ACPI-enabled platform so far. It seems
a confusion with extcon-intel-int3496 which does support ACPI-enabled
platforms.

Revert commit 942c7924a51e to avoid any confusion in the future.

Cc: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon-usb-gpio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index d47573a31e17..9c925b05b7aa 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -26,7 +26,6 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
-#include <linux/acpi.h>
 #include <linux/pinctrl/consumer.h>
 
 #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
@@ -111,7 +110,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
 	struct usb_extcon_info *info;
 	int ret;
 
-	if (!np && !ACPI_HANDLE(dev))
+	if (!np)
 		return -EINVAL;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
-- 
2.11.0

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Chanwoo Choi]

Hi,

On 2017년 03월 22일 03:37, Andy Shevchenko wrote:
> The commit 942c7924a51e introduced a check for ACPI handle for the
> device that never appears on any ACPI-enabled platform so far. It seems
> a confusion with extcon-intel-int3496 which does support ACPI-enabled
> platforms.

Only for the reason that there is no any usecase until now,
and remove the confusion between extcon-usb-gpio and extcon-intel-int3496.
Should we revert it? 

I think that both extcon-usb-gpio and extcon-intel-int3496
driver are not same operation perfectly. Also, the filename
of extcon-intel-int3496 has specific name. Instead, extcon-usb-gpio.c
is more common device driver.

Can the extcon-intel-int3496.c support the everything on acpi side?

> 
> Revert commit 942c7924a51e to avoid any confusion in the future.
> 
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon-usb-gpio.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index d47573a31e17..9c925b05b7aa 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -26,7 +26,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
> -#include <linux/acpi.h>
>  #include <linux/pinctrl/consumer.h>
>  
>  #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
> @@ -111,7 +110,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  	struct usb_extcon_info *info;
>  	int ret;
>  
> -	if (!np && !ACPI_HANDLE(dev))
> +	if (!np)
>  		return -EINVAL;
>  
>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Andy Shevchenko]

On Wed, 2017-03-22 at 10:14 +0900, Chanwoo Choi wrote:
> Hi,
> 
> On 2017년 03월 22일 03:37, Andy Shevchenko wrote:
> > The commit 942c7924a51e introduced a check for ACPI handle for the
> > device that never appears on any ACPI-enabled platform so far. It
> > seems
> > a confusion with extcon-intel-int3496 which does support ACPI-
> > enabled
> > platforms.
> 
> Only for the reason that there is no any usecase until now,
> and remove the confusion between extcon-usb-gpio and extcon-intel-
> int3496.
> Should we revert it? 


> 
> I think that both extcon-usb-gpio and extcon-intel-int3496
> driver are not same operation perfectly. Also, the filename
> of extcon-intel-int3496 has specific name. Instead, extcon-usb-gpio.c
> is more common device driver.
> 
> Can the extcon-intel-int3496.c support the everything on acpi side?

For my understanding we have the only driver for now for USB mux in the
kernel for ACPI-enabled platforms.

Besides confusion, it makes harder to fix a real bugs in at least GPIO
ACPI library since we need to amend any user of it first. While
confusion is here, I can't do anything to not possible break the
functionality of the driver in a real use case if any (I doubt there is 
any in this particular case).

So, my opinion here is "yes, we should revert it until we have a
confirmation that there is a product which is using this among with
ACPI" (which I doubt ever exists).

> 
> > 
> > Revert commit 942c7924a51e to avoid any confusion in the future.
> > 
> > Cc: Lu Baolu <baolu.lu@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/extcon/extcon-usb-gpio.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/extcon/extcon-usb-gpio.c
> > b/drivers/extcon/extcon-usb-gpio.c
> > index d47573a31e17..9c925b05b7aa 100644
> > --- a/drivers/extcon/extcon-usb-gpio.c
> > +++ b/drivers/extcon/extcon-usb-gpio.c
> > @@ -26,7 +26,6 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> >  #include <linux/workqueue.h>
> > -#include <linux/acpi.h>
> >  #include <linux/pinctrl/consumer.h>
> >  
> >  #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
> > @@ -111,7 +110,7 @@ static int usb_extcon_probe(struct
> > platform_device *pdev)
> >  	struct usb_extcon_info *info;
> >  	int ret;
> >  
> > -	if (!np && !ACPI_HANDLE(dev))
> > +	if (!np)
> >  		return -EINVAL;
> >  
> >  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> > 
> 
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Chanwoo Choi]

Hi,

On 2017년 03월 22일 22:09, Andy Shevchenko wrote:
> On Wed, 2017-03-22 at 10:14 +0900, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2017년 03월 22일 03:37, Andy Shevchenko wrote:
>>> The commit 942c7924a51e introduced a check for ACPI handle for the
>>> device that never appears on any ACPI-enabled platform so far. It
>>> seems
>>> a confusion with extcon-intel-int3496 which does support ACPI-
>>> enabled
>>> platforms.
>>
>> Only for the reason that there is no any usecase until now,
>> and remove the confusion between extcon-usb-gpio and extcon-intel-
>> int3496.
>> Should we revert it? 
> 
> 
>>
>> I think that both extcon-usb-gpio and extcon-intel-int3496
>> driver are not same operation perfectly. Also, the filename
>> of extcon-intel-int3496 has specific name. Instead, extcon-usb-gpio.c
>> is more common device driver.
>>
>> Can the extcon-intel-int3496.c support the everything on acpi side?
> 
> For my understanding we have the only driver for now for USB mux in the
> kernel for ACPI-enabled platforms.
> 
> Besides confusion, it makes harder to fix a real bugs in at least GPIO
> ACPI library since we need to amend any user of it first. While
> confusion is here, I can't do anything to not possible break the
> functionality of the driver in a real use case if any (I doubt there is 
> any in this particular case).
> 
> So, my opinion here is "yes, we should revert it until we have a
> confirmation that there is a product which is using this among with
> ACPI" (which I doubt ever exists).

Because you told me there was not any use case of extcon-usb-gpioc.c
on acpi side. But, I think that it is not enough as the reason.

Because I already mentioned,
1.
"The both extcon-usb-gpio and extcon-intel-int3496 driver
are not same operation perfectly." It two driver are same operation
and there is no use case on acpi side, I may agree your suggestion.
But, in this case, they are different between two drivers.

2.
Also, extcon-intel-int3496 has the specific name 'int3496'.
I think that it only depends on the specific device driver on acpi side.
I don't think it cover all of use case on acpi side.

> 
>>
>>>
>>> Revert commit 942c7924a51e to avoid any confusion in the future.
>>>
>>> Cc: Lu Baolu <baolu.lu@linux.intel.com>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> ---
>>>  drivers/extcon/extcon-usb-gpio.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-usb-gpio.c
>>> b/drivers/extcon/extcon-usb-gpio.c
>>> index d47573a31e17..9c925b05b7aa 100644
>>> --- a/drivers/extcon/extcon-usb-gpio.c
>>> +++ b/drivers/extcon/extcon-usb-gpio.c
>>> @@ -26,7 +26,6 @@
>>>  #include <linux/platform_device.h>
>>>  #include <linux/slab.h>
>>>  #include <linux/workqueue.h>
>>> -#include <linux/acpi.h>
>>>  #include <linux/pinctrl/consumer.h>
>>>  
>>>  #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
>>> @@ -111,7 +110,7 @@ static int usb_extcon_probe(struct
>>> platform_device *pdev)
>>>  	struct usb_extcon_info *info;
>>>  	int ret;
>>>  
>>> -	if (!np && !ACPI_HANDLE(dev))
>>> +	if (!np)
>>>  		return -EINVAL;
>>>  
>>>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>>>
>>
>>
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Andy Shevchenko]

On Fri, 2017-03-24 at 20:03 +0900, Chanwoo Choi wrote:
> On 2017년 03월 22일 22:09, Andy Shevchenko wrote:
> > On Wed, 2017-03-22 at 10:14 +0900, Chanwoo Choi wrote:
> > > On 2017년 03월 22일 03:37, Andy Shevchenko wrote:

> > > > The commit 942c7924a51e introduced a check for ACPI handle for
> > > > the
> > > > device that never appears on any ACPI-enabled platform so far.
> > > > It
> > > > seems
> > > > a confusion with extcon-intel-int3496 which does support ACPI-
> > > > enabled
> > > > platforms.
> > > 
> > > Only for the reason that there is no any usecase until now,
> > > and remove the confusion between extcon-usb-gpio and extcon-intel-
> > > int3496.
> > > Should we revert it? 
> > 
> > 
> > > 
> > > I think that both extcon-usb-gpio and extcon-intel-int3496
> > > driver are not same operation perfectly. Also, the filename
> > > of extcon-intel-int3496 has specific name. Instead, extcon-usb-
> > > gpio.c
> > > is more common device driver.
> > > 
> > > Can the extcon-intel-int3496.c support the everything on acpi
> > > side?
> > 
> > For my understanding we have the only driver for now for USB mux in
> > the
> > kernel for ACPI-enabled platforms.
> > 
> > Besides confusion, it makes harder to fix a real bugs in at least
> > GPIO
> > ACPI library since we need to amend any user of it first. While
> > confusion is here, I can't do anything to not possible break the
> > functionality of the driver in a real use case if any (I doubt there
> > is 
> > any in this particular case).
> > 
> > So, my opinion here is "yes, we should revert it until we have a
> > confirmation that there is a product which is using this among with
> > ACPI" (which I doubt ever exists).
> 
> Because you told me there was not any use case of extcon-usb-gpioc.c
> on acpi side. But, I think that it is not enough as the reason.
> 
> Because I already mentioned,
> 1.
> "The both extcon-usb-gpio and extcon-intel-int3496 driver
> are not same operation perfectly." It two driver are same operation
> and there is no use case on acpi side, I may agree your suggestion.
> But, in this case, they are different between two drivers.
> 
> 2.
> Also, extcon-intel-int3496 has the specific name 'int3496'.
> I think that it only depends on the specific device driver on acpi
> side.
> I don't think it cover all of use case on acpi side.

Just one question: is there *real* existing device where ACPI table
contains something related to extcon-usb-gpio? 

I'm pretty sure the answer is no. Moreover, Lu pointed me out to the
series which tried to update the driver in question to support int3496.
Though it comes as a separate driver, thus that series was abandoned
IIUC.

I really don't care if some dead confusing code will be left in some
poor driver, at the end it's not my call.

P.S. We already spent enough time making a mountain out of a molehill. I
rest my case.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Chanwoo Choi]

Hi Lu Baolu,

On 2017년 03월 24일 20:24, Andy Shevchenko wrote:
> On Fri, 2017-03-24 at 20:03 +0900, Chanwoo Choi wrote:
>> On 2017년 03월 22일 22:09, Andy Shevchenko wrote:
>>> On Wed, 2017-03-22 at 10:14 +0900, Chanwoo Choi wrote:
>>>> On 2017년 03월 22일 03:37, Andy Shevchenko wrote:
> 
>>>>> The commit 942c7924a51e introduced a check for ACPI handle for
>>>>> the
>>>>> device that never appears on any ACPI-enabled platform so far.
>>>>> It
>>>>> seems
>>>>> a confusion with extcon-intel-int3496 which does support ACPI-
>>>>> enabled
>>>>> platforms.
>>>>
>>>> Only for the reason that there is no any usecase until now,
>>>> and remove the confusion between extcon-usb-gpio and extcon-intel-
>>>> int3496.
>>>> Should we revert it? 
>>>
>>>
>>>>
>>>> I think that both extcon-usb-gpio and extcon-intel-int3496
>>>> driver are not same operation perfectly. Also, the filename
>>>> of extcon-intel-int3496 has specific name. Instead, extcon-usb-
>>>> gpio.c
>>>> is more common device driver.
>>>>
>>>> Can the extcon-intel-int3496.c support the everything on acpi
>>>> side?
>>>
>>> For my understanding we have the only driver for now for USB mux in
>>> the
>>> kernel for ACPI-enabled platforms.
>>>
>>> Besides confusion, it makes harder to fix a real bugs in at least
>>> GPIO
>>> ACPI library since we need to amend any user of it first. While
>>> confusion is here, I can't do anything to not possible break the
>>> functionality of the driver in a real use case if any (I doubt there
>>> is 
>>> any in this particular case).
>>>
>>> So, my opinion here is "yes, we should revert it until we have a
>>> confirmation that there is a product which is using this among with
>>> ACPI" (which I doubt ever exists).
>>
>> Because you told me there was not any use case of extcon-usb-gpioc.c
>> on acpi side. But, I think that it is not enough as the reason.
>>
>> Because I already mentioned,
>> 1.
>> "The both extcon-usb-gpio and extcon-intel-int3496 driver
>> are not same operation perfectly." It two driver are same operation
>> and there is no use case on acpi side, I may agree your suggestion.
>> But, in this case, they are different between two drivers.
>>
>> 2.
>> Also, extcon-intel-int3496 has the specific name 'int3496'.
>> I think that it only depends on the specific device driver on acpi
>> side.
>> I don't think it cover all of use case on acpi side.
> 
> Just one question: is there *real* existing device where ACPI table
> contains something related to extcon-usb-gpio? 
> 
> I'm pretty sure the answer is no. Moreover, Lu pointed me out to the
> series which tried to update the driver in question to support int3496.
> Though it comes as a separate driver, thus that series was abandoned
> IIUC.
> 
> I really don't care if some dead confusing code will be left in some
> poor driver, at the end it's not my call.
> 
> P.S. We already spent enough time making a mountain out of a molehill. I
> rest my case.
> 

OK. Just I want to receive the reply from Lu Baolu.

In the "extcon-usb-gpio ACPI support" mail thread,
I understood that Lu Baolu said that the related patches were abandoned.

To Lu Baolu,
Don't you ever use the extcon-usb-gpio.c in the future on acpi side?
If you agree it, I'll revert it.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Lu Baolu]

Hi,

On 03/24/2017 07:47 PM, Chanwoo Choi wrote:
> Hi Lu Baolu,
>
> On 2017년 03월 24일 20:24, Andy Shevchenko wrote:
>> On Fri, 2017-03-24 at 20:03 +0900, Chanwoo Choi wrote:
>>> On 2017년 03월 22일 22:09, Andy Shevchenko wrote:
>>>> On Wed, 2017-03-22 at 10:14 +0900, Chanwoo Choi wrote:
>>>>> On 2017년 03월 22일 03:37, Andy Shevchenko wrote:
>>>>>> The commit 942c7924a51e introduced a check for ACPI handle for
>>>>>> the
>>>>>> device that never appears on any ACPI-enabled platform so far.
>>>>>> It
>>>>>> seems
>>>>>> a confusion with extcon-intel-int3496 which does support ACPI-
>>>>>> enabled
>>>>>> platforms.
>>>>> Only for the reason that there is no any usecase until now,
>>>>> and remove the confusion between extcon-usb-gpio and extcon-intel-
>>>>> int3496.
>>>>> Should we revert it? 
>>>>
>>>>> I think that both extcon-usb-gpio and extcon-intel-int3496
>>>>> driver are not same operation perfectly. Also, the filename
>>>>> of extcon-intel-int3496 has specific name. Instead, extcon-usb-
>>>>> gpio.c
>>>>> is more common device driver.
>>>>>
>>>>> Can the extcon-intel-int3496.c support the everything on acpi
>>>>> side?
>>>> For my understanding we have the only driver for now for USB mux in
>>>> the
>>>> kernel for ACPI-enabled platforms.
>>>>
>>>> Besides confusion, it makes harder to fix a real bugs in at least
>>>> GPIO
>>>> ACPI library since we need to amend any user of it first. While
>>>> confusion is here, I can't do anything to not possible break the
>>>> functionality of the driver in a real use case if any (I doubt there
>>>> is 
>>>> any in this particular case).
>>>>
>>>> So, my opinion here is "yes, we should revert it until we have a
>>>> confirmation that there is a product which is using this among with
>>>> ACPI" (which I doubt ever exists).
>>> Because you told me there was not any use case of extcon-usb-gpioc.c
>>> on acpi side. But, I think that it is not enough as the reason.
>>>
>>> Because I already mentioned,
>>> 1.
>>> "The both extcon-usb-gpio and extcon-intel-int3496 driver
>>> are not same operation perfectly." It two driver are same operation
>>> and there is no use case on acpi side, I may agree your suggestion.
>>> But, in this case, they are different between two drivers.
>>>
>>> 2.
>>> Also, extcon-intel-int3496 has the specific name 'int3496'.
>>> I think that it only depends on the specific device driver on acpi
>>> side.
>>> I don't think it cover all of use case on acpi side.
>> Just one question: is there *real* existing device where ACPI table
>> contains something related to extcon-usb-gpio? 
>>
>> I'm pretty sure the answer is no. Moreover, Lu pointed me out to the
>> series which tried to update the driver in question to support int3496.
>> Though it comes as a separate driver, thus that series was abandoned
>> IIUC.
>>
>> I really don't care if some dead confusing code will be left in some
>> poor driver, at the end it's not my call.
>>
>> P.S. We already spent enough time making a mountain out of a molehill. I
>> rest my case.
>>
> OK. Just I want to receive the reply from Lu Baolu.
>
> In the "extcon-usb-gpio ACPI support" mail thread,
> I understood that Lu Baolu said that the related patches were abandoned.
>
> To Lu Baolu,
> Don't you ever use the extcon-usb-gpio.c in the future on acpi side?
> If you agree it, I'll revert it.
>

I will not use extcon-usb-gpio.c in the future on acpi side AFAICS.

Best regards,
Lu Baolu

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Chanwoo Choi]

Hi Andy,

On 2017년 03월 22일 03:37, Andy Shevchenko wrote:
> The commit 942c7924a51e introduced a check for ACPI handle for the
> device that never appears on any ACPI-enabled platform so far. It seems
> a confusion with extcon-intel-int3496 which does support ACPI-enabled
> platforms.
> 
> Revert commit 942c7924a51e to avoid any confusion in the future.
> 
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon-usb-gpio.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 

Applied it on extcon-fixes branch.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1] Revert "extcon: usb-gpio: add support for ACPI gpio interface"

[Andy Shevchenko]

On Mon, 2017-03-27 at 13:18 +0900, Chanwoo Choi wrote:
> Hi Andy,
> 
> On 2017년 03월 22일 03:37, Andy Shevchenko wrote:
> > The commit 942c7924a51e introduced a check for ACPI handle for the
> > device that never appears on any ACPI-enabled platform so far. It
> > seems
> > a confusion with extcon-intel-int3496 which does support ACPI-
> > enabled
> > platforms.
> > 
> > Revert commit 942c7924a51e to avoid any confusion in the future.
> > 
> > Cc: Lu Baolu <baolu.lu@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/extcon/extcon-usb-gpio.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> 
> Applied it on extcon-fixes branch.

Thanks!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy