Message ID | 5948633.my3nAG6hRz@wasted.cogentembedded.com |
---|---|
State | Not Applicable, archived |
Headers | show |
Hello. On 12/20/2015 05:31 PM, Chanwoo Choi wrote: > This patch depend on GPIOLIB configuration as following: > I modified it with following diff and applied it. > > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig > index ba4db7d..3d89e60 100644 > --- a/drivers/extcon/Kconfig > +++ b/drivers/extcon/Kconfig > @@ -54,6 +54,7 @@ config EXTCON_MAX14577 > > config EXTCON_MAX3355 > tristate "Maxim MAX3355 USB OTG EXTCON Support" > + depends on GPIOLIB || COMPILE_TEST If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? And no, it shouldn't depend on gpiolib. It has empty stubs for the case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look into it. [...] > Thanks, > Chanwoo Choi MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 2015년 12월 21일 02:15, Sergei Shtylyov wrote: > Hello. > > On 12/20/2015 05:31 PM, Chanwoo Choi wrote: > >> This patch depend on GPIOLIB configuration as following: >> I modified it with following diff and applied it. >> >> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >> index ba4db7d..3d89e60 100644 >> --- a/drivers/extcon/Kconfig >> +++ b/drivers/extcon/Kconfig >> @@ -54,6 +54,7 @@ config EXTCON_MAX14577 >> >> config EXTCON_MAX3355 >> tristate "Maxim MAX3355 USB OTG EXTCON Support" >> + depends on GPIOLIB || COMPILE_TEST > > If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? > And no, it shouldn't depend on gpiolib. It has empty stubs for the case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look into it. Yes. When GPIOLIB is disabled, the build issue don't happen. because include/linux/gpio/consumer.h implement the dummy function for all gpio functions if CONFIG_GPIOLIB is disabled. For correct operation of max3355, you should add the dependency to the extcon-max3355.c driver. This driver use the GPIO library certainly. COMPILE_TEST is used for just build test. You can see the detailed data[1]. [1] https://lkml.org/lkml/2013/5/22/155 Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 12/21/2015 5:38 AM, Chanwoo Choi wrote: >>> This patch depend on GPIOLIB configuration as following: >>> I modified it with following diff and applied it. >>> >>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>> index ba4db7d..3d89e60 100644 >>> --- a/drivers/extcon/Kconfig >>> +++ b/drivers/extcon/Kconfig >>> @@ -54,6 +54,7 @@ config EXTCON_MAX14577 >>> >>> config EXTCON_MAX3355 >>> tristate "Maxim MAX3355 USB OTG EXTCON Support" >>> + depends on GPIOLIB || COMPILE_TEST >> >> If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? >> And no, it shouldn't depend on gpiolib. It has empty stubs for the case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look into it. > > Yes. When GPIOLIB is disabled, the build issue don't happen. What? It surely does happen! > because include/linux/gpio/consumer.h implement the dummy function > for all gpio functions if CONFIG_GPIOLIB is disabled. Linus W. advised to #include this header explicitly -- I'll try and post. > For correct operation of max3355, you should add the dependency > to the extcon-max3355.c driver. This driver use the GPIO library > certainly. I disagree. The driver will just cease to load in this case. I don't see why we need such dependency. Only compilation time dependencies should be specified, I think. > COMPILE_TEST is used for just build test. You can see the detailed data[1]. > [1] https://lkml.org/lkml/2013/5/22/155 I know. Re-read my question please. > Thanks, > Chanwoo Choi MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 12/21/2015 02:01 PM, Sergei Shtylyov wrote: >>>> This patch depend on GPIOLIB configuration as following: >>>> I modified it with following diff and applied it. >>>> >>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>>> index ba4db7d..3d89e60 100644 >>>> --- a/drivers/extcon/Kconfig >>>> +++ b/drivers/extcon/Kconfig >>>> @@ -54,6 +54,7 @@ config EXTCON_MAX14577 >>>> >>>> config EXTCON_MAX3355 >>>> tristate "Maxim MAX3355 USB OTG EXTCON Support" >>>> + depends on GPIOLIB || COMPILE_TEST >>> >>> If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? >>> And no, it shouldn't depend on gpiolib. It has empty stubs for the case >>> of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, >>> I'll look into it. >> >> Yes. When GPIOLIB is disabled, the build issue don't happen. > > What? It surely does happen! > >> because include/linux/gpio/consumer.h implement the dummy function >> for all gpio functions if CONFIG_GPIOLIB is disabled. > > Linus W. advised to #include this header explicitly -- I'll try and post. I see you already #include'd it, thanks. But in that case, <linux/gpio.h> doesn't seem necessary. >> Thanks, >> Chanwoo Choi MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015년 12월 21일 20:01, Sergei Shtylyov wrote: > Hello. > > On 12/21/2015 5:38 AM, Chanwoo Choi wrote: > >>>> This patch depend on GPIOLIB configuration as following: >>>> I modified it with following diff and applied it. >>>> >>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>>> index ba4db7d..3d89e60 100644 >>>> --- a/drivers/extcon/Kconfig >>>> +++ b/drivers/extcon/Kconfig >>>> @@ -54,6 +54,7 @@ config EXTCON_MAX14577 >>>> >>>> config EXTCON_MAX3355 >>>> tristate "Maxim MAX3355 USB OTG EXTCON Support" >>>> + depends on GPIOLIB || COMPILE_TEST >>> >>> If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? >>> And no, it shouldn't depend on gpiolib. It has empty stubs for the case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look into it. >> >> Yes. When GPIOLIB is disabled, the build issue don't happen. > > What? It surely does happen! hmm.... Sure. you need to check the include/linux/gpio/consumer.h. Because of build error happen, you miss to include the "linux/gpio/consumer.h" header file in extcon-max3355.c. Please test it for enough time. > >> because include/linux/gpio/consumer.h implement the dummy function >> for all gpio functions if CONFIG_GPIOLIB is disabled. > > Linus W. advised to #include this header explicitly -- I'll try and post. Don't necessary. I already updated it including the "include/linux/gpio/consumer.h". > >> For correct operation of max3355, you should add the dependency >> to the extcon-max3355.c driver. This driver use the GPIO library >> certainly. > > I disagree. The driver will just cease to load in this case. I don't see why we need such dependency. Only compilation time dependencies should be > specified, I think. This driver have to depend on GPIOLIB. Why are you disagreeing the COMPILE_TEST dependency? It is just compile test without anything. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 12/22/2015 4:13 AM, Chanwoo Choi wrote: >>>>> This patch depend on GPIOLIB configuration as following: >>>>> I modified it with following diff and applied it. >>>>> >>>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>>>> index ba4db7d..3d89e60 100644 >>>>> --- a/drivers/extcon/Kconfig >>>>> +++ b/drivers/extcon/Kconfig >>>>> @@ -54,6 +54,7 @@ config EXTCON_MAX14577 >>>>> >>>>> config EXTCON_MAX3355 >>>>> tristate "Maxim MAX3355 USB OTG EXTCON Support" >>>>> + depends on GPIOLIB || COMPILE_TEST >>>> >>>> If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? >>>> And no, it shouldn't depend on gpiolib. It has empty stubs for the case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look into it. >>> >>> Yes. When GPIOLIB is disabled, the build issue don't happen. >> >> What? It surely does happen! > > hmm.... > Sure. you need to check the include/linux/gpio/consumer.h. > > Because of build error happen, you miss to include the "linux/gpio/consumer.h" > header file in extcon-max3355.c. Please test it for enough time. Yes, with this file #include'd, it build fine now. >>> because include/linux/gpio/consumer.h implement the dummy function >>> for all gpio functions if CONFIG_GPIOLIB is disabled. >> >> Linus W. advised to #include this header explicitly -- I'll try and post. > > Don't necessary. I already updated it including the "include/linux/gpio/consumer.h". I saw that, yes. >>> For correct operation of max3355, you should add the dependency >>> to the extcon-max3355.c driver. This driver use the GPIO library >>> certainly. >> >> I disagree. The driver will just cease to load in this case. I don't see why we need such dependency. Only compilation time dependencies should be >> specified, I think. > > This driver have to depend on GPIOLIB. > Why are you disagreeing the COMPILE_TEST dependency? It is just compile test > without anything. I agree now. I still disagree about the gpiolib dependency though. MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015년 12월 22일 20:15, Sergei Shtylyov wrote: > Hello. > > On 12/22/2015 4:13 AM, Chanwoo Choi wrote: > >>>>>> This patch depend on GPIOLIB configuration as following: >>>>>> I modified it with following diff and applied it. >>>>>> >>>>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>>>>> index ba4db7d..3d89e60 100644 >>>>>> --- a/drivers/extcon/Kconfig >>>>>> +++ b/drivers/extcon/Kconfig >>>>>> @@ -54,6 +54,7 @@ config EXTCON_MAX14577 >>>>>> >>>>>> config EXTCON_MAX3355 >>>>>> tristate "Maxim MAX3355 USB OTG EXTCON Support" >>>>>> + depends on GPIOLIB || COMPILE_TEST >>>>> >>>>> If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? >>>>> And no, it shouldn't depend on gpiolib. It has empty stubs for the case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look into it. >>>> >>>> Yes. When GPIOLIB is disabled, the build issue don't happen. >>> >>> What? It surely does happen! >> >> hmm.... >> Sure. you need to check the include/linux/gpio/consumer.h. >> >> Because of build error happen, you miss to include the "linux/gpio/consumer.h" >> header file in extcon-max3355.c. Please test it for enough time. > > Yes, with this file #include'd, it build fine now. > >>>> because include/linux/gpio/consumer.h implement the dummy function >>>> for all gpio functions if CONFIG_GPIOLIB is disabled. >>> >>> Linus W. advised to #include this header explicitly -- I'll try and post. >> >> Don't necessary. I already updated it including the "include/linux/gpio/consumer.h". > > I saw that, yes. > >>>> For correct operation of max3355, you should add the dependency >>>> to the extcon-max3355.c driver. This driver use the GPIO library >>>> certainly. >>> >>> I disagree. The driver will just cease to load in this case. I don't see why we need such dependency. Only compilation time dependencies should be >>> specified, I think. >> >> This driver have to depend on GPIOLIB. >> Why are you disagreeing the COMPILE_TEST dependency? It is just compile test >> without anything. > > I agree now. I still disagree about the gpiolib dependency though. If gpiolib is disabled, extcon-max3355.c might not operate it correctly. Just this driver could be built without operation because gpiolib function will not do the any behavior. I think that it is not too much problem. I should send the pull request within this week. If you want to need more discussion of extcon-max3355.c, I will not include it on pull request for v4.5 because there is issue. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 12/23/2015 05:17 AM, Chanwoo Choi wrote: >>>>>>> This patch depend on GPIOLIB configuration as following: >>>>>>> I modified it with following diff and applied it. >>>>>>> >>>>>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>>>>>> index ba4db7d..3d89e60 100644 >>>>>>> --- a/drivers/extcon/Kconfig >>>>>>> +++ b/drivers/extcon/Kconfig >>>>>>> @@ -54,6 +54,7 @@ config EXTCON_MAX14577 >>>>>>> >>>>>>> config EXTCON_MAX3355 >>>>>>> tristate "Maxim MAX3355 USB OTG EXTCON Support" >>>>>>> + depends on GPIOLIB || COMPILE_TEST >>>>>> >>>>>> If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST? >>>>>> And no, it shouldn't depend on gpiolib. It has empty stubs for the case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look into it. >>>>> >>>>> Yes. When GPIOLIB is disabled, the build issue don't happen. >>>> >>>> What? It surely does happen! >>> >>> hmm.... >>> Sure. you need to check the include/linux/gpio/consumer.h. >>> >>> Because of build error happen, you miss to include the "linux/gpio/consumer.h" >>> header file in extcon-max3355.c. Please test it for enough time. >> >> Yes, with this file #include'd, it build fine now. >> >>>>> because include/linux/gpio/consumer.h implement the dummy function >>>>> for all gpio functions if CONFIG_GPIOLIB is disabled. >>>> >>>> Linus W. advised to #include this header explicitly -- I'll try and post. >>> >>> Don't necessary. I already updated it including the "include/linux/gpio/consumer.h". >> >> I saw that, yes. >> >>>>> For correct operation of max3355, you should add the dependency >>>>> to the extcon-max3355.c driver. This driver use the GPIO library >>>>> certainly. >>>> >>>> I disagree. The driver will just cease to load in this case. I don't see why we need such dependency. Only compilation time dependencies should be >>>> specified, I think. >>> >>> This driver have to depend on GPIOLIB. >>> Why are you disagreeing the COMPILE_TEST dependency? It is just compile test >>> without anything. >> >> I agree now. I still disagree about the gpiolib dependency though. > > If gpiolib is disabled, extcon-max3355.c might not operate it correctly. It'll just fail the probe, that's all. > Just this driver could be built without operation because gpiolib function > will not do the any behavior. devm_gpiod_get() will just fail with -ENOSYS. > I think that it is not too much problem. I should send the pull request within this week. > If you want to need more discussion of extcon-max3355.c, > I will not include it on pull request for v4.5 because there is issue. No, please include it into the pull request. MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: extcon/Documentation/devicetree/bindings/extcon/extcon-max3355.txt =================================================================== --- /dev/null +++ extcon/Documentation/devicetree/bindings/extcon/extcon-max3355.txt @@ -0,0 +1,21 @@ +Maxim Integrated MAX3355 USB OTG chip +------------------------------------- + +MAX3355 integrates a charge pump and comparators to enable a system with an +integrated USB OTG dual-role transceiver to function as a USB OTG dual-role +device. + +Required properties: +- compatible: should be "maxim,max3355"; +- maxim,shdn-gpios: should contain a phandle and GPIO specifier for the GPIO pin + connected to the MAX3355's SHDN# pin; +- id-gpios: should contain a phandle and GPIO specifier for the GPIO pin + connected to the MAX3355's ID_OUT pin. + +Example: + + usb-otg { + compatible = "maxim,max3355"; + maxim,shdn-gpios = <&gpio2 4 GPIO_ACTIVE_LOW>; + id-gpios = <&gpio5 31 GPIO_ACTIVE_HIGH>; + }; Index: extcon/drivers/extcon/Kconfig =================================================================== --- extcon.orig/drivers/extcon/Kconfig +++ extcon/drivers/extcon/Kconfig @@ -52,6 +52,14 @@ config EXTCON_MAX14577 Maxim MAX14577/77836. The MAX14577/77836 MUIC is a USB port accessory detector and switch. +config EXTCON_MAX3355 + tristate "Maxim MAX3355 USB OTG EXTCON Support" + help + If you say yes here you get support for the USB OTG role detection by + MAX3355. The MAX3355 chip integrates a charge pump and comparators to + enable a system with an integrated USB OTG dual-role transceiver to + function as an USB OTG dual-role device. + config EXTCON_MAX77693 tristate "Maxim MAX77693 EXTCON Support" depends on MFD_MAX77693 && INPUT Index: extcon/drivers/extcon/Makefile =================================================================== --- extcon.orig/drivers/extcon/Makefile +++ extcon/drivers/extcon/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_EXTCON_ARIZONA) += extcon-a obj-$(CONFIG_EXTCON_AXP288) += extcon-axp288.o obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o obj-$(CONFIG_EXTCON_MAX14577) += extcon-max14577.o +obj-$(CONFIG_EXTCON_MAX3355) += extcon-max3355.o obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o obj-$(CONFIG_EXTCON_MAX77843) += extcon-max77843.o obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o Index: extcon/drivers/extcon/extcon-max3355.c =================================================================== --- /dev/null +++ extcon/drivers/extcon/extcon-max3355.c @@ -0,0 +1,150 @@ +/* + * Maxim Integrated MAX3355 USB OTG chip extcon driver + * + * Copyright (C) 2014-2015 Cogent Embedded, Inc. + * Author: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + */ + +#include <linux/extcon.h> +#include <linux/gpio.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/workqueue.h> + +struct max3355_data { + struct extcon_dev *edev; + struct gpio_desc *id_gpiod; + struct gpio_desc *shdn_gpiod; +}; + +static const unsigned int max3355_cable[] = { + EXTCON_USB, + EXTCON_USB_HOST, + EXTCON_NONE, +}; + +static irqreturn_t max3355_id_irq(int irq, void *dev_id) +{ + struct max3355_data *data = dev_id; + int id = gpiod_get_value_cansleep(data->id_gpiod); + + if (id) { + /* + * ID = 1 means USB HOST cable detached. + * As we don't have event for USB peripheral cable attached, + * we simulate USB peripheral attach here. + */ + extcon_set_cable_state_(data->edev, EXTCON_USB_HOST, false); + extcon_set_cable_state_(data->edev, EXTCON_USB, true); + } else { + /* + * ID = 0 means USB HOST cable attached. + * As we don't have event for USB peripheral cable detached, + * we simulate USB peripheral detach here. + */ + extcon_set_cable_state_(data->edev, EXTCON_USB, false); + extcon_set_cable_state_(data->edev, EXTCON_USB_HOST, true); + } + + return IRQ_HANDLED; +} + +static int max3355_probe(struct platform_device *pdev) +{ + struct max3355_data *data; + struct gpio_desc *gpiod; + int irq, err; + + data = devm_kzalloc(&pdev->dev, sizeof(struct max3355_data), + GFP_KERNEL); + if (!data) + return -ENOMEM; + + gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN); + if (IS_ERR(gpiod)) { + dev_err(&pdev->dev, "failed to get ID_OUT GPIO\n"); + return PTR_ERR(gpiod); + } + data->id_gpiod = gpiod; + + gpiod = devm_gpiod_get(&pdev->dev, "maxim,shdn", GPIOD_OUT_HIGH); + if (IS_ERR(gpiod)) { + dev_err(&pdev->dev, "failed to get SHDN# GPIO\n"); + return PTR_ERR(gpiod); + } + data->shdn_gpiod = gpiod; + + data->edev = devm_extcon_dev_allocate(&pdev->dev, max3355_cable); + if (IS_ERR(data->edev)) { + dev_err(&pdev->dev, "failed to allocate extcon device\n"); + return PTR_ERR(data->edev); + } + + err = devm_extcon_dev_register(&pdev->dev, data->edev); + if (err < 0) { + dev_err(&pdev->dev, "failed to register extcon device\n"); + return err; + } + + irq = gpiod_to_irq(data->id_gpiod); + if (irq < 0) { + dev_err(&pdev->dev, "failed to translate ID_OUT GPIO to IRQ\n"); + return irq; + } + + err = devm_request_threaded_irq(&pdev->dev, irq, NULL, max3355_id_irq, + IRQF_ONESHOT | IRQF_NO_SUSPEND | + IRQF_TRIGGER_RISING | + IRQF_TRIGGER_FALLING, + pdev->name, data); + if (err < 0) { + dev_err(&pdev->dev, "failed to request ID_OUT IRQ\n"); + return err; + } + + platform_set_drvdata(pdev, data); + + /* Perform initial detection */ + max3355_id_irq(irq, data); + + return 0; +} + +static int max3355_remove(struct platform_device *pdev) +{ + struct max3355_data *data = platform_get_drvdata(pdev); + + gpiod_set_value_cansleep(data->shdn_gpiod, 0); + + return 0; +} + +static const struct of_device_id max3355_match_table[] = { + { .compatible = "maxim,max3355", }, + { } +}; +MODULE_DEVICE_TABLE(of, max3355_match_table); + +static struct platform_driver max3355_driver = { + .probe = max3355_probe, + .remove = max3355_remove, + .driver = { + .name = "extcon-max3355", + .of_match_table = max3355_match_table, + }, +}; + +module_platform_driver(max3355_driver); + +MODULE_AUTHOR("Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>"); +MODULE_DESCRIPTION("Maxim MAX3355 extcon driver"); +MODULE_LICENSE("GPL v2");