From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S942224AbcLWSfC (ORCPT ); Fri, 23 Dec 2016 13:35:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45314 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S942144AbcLWSfB (ORCPT ); Fri, 23 Dec 2016 13:35:01 -0500 Subject: Re: [PATCH v2] extcon: int3496: Add Intel INT3496 ACPI device extcon driver To: cwchoi00@gmail.com References: <20161221112025.13600-1-hdegoede@redhat.com> Cc: MyungJoo Ham , Chanwoo Choi , Chen-Yu Tsai , "russianneuromancer @ ya . ru" , linux-kernel , David Cohen From: Hans de Goede Message-ID: <40a0cd61-784e-1795-21d7-255f083570e0@redhat.com> Date: Fri, 23 Dec 2016 19:34:56 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 23 Dec 2016 18:35:01 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 23-12-16 18:38, Chanwoo Choi wrote: > Hi Hans, > > I have two minor comment on below. > But, after fixing it by myself, Applied it on extcon-next branch for v4.11. Thank you for fixing those up. Regards, Hans > > 2016-12-21 20:20 GMT+09:00 Hans de Goede : >> From: David Cohen >> >> Add an extcon driver for USB OTG ports controlled by an Intel INT3496 >> ACPI device (e.g. Baytrail, Cherrytrail devices). >> >> Signed-off-by: David Cohen >> [hdgoede@redhat.com: Port to current kernel, cleanup, submit upstream] >> [hdgoede@redhat.com: Add Documentation/extcon/intel-int3496.txt] >> Signed-off-by: Hans de Goede >> --- >> Changes in v2: >> -Rename to extcon-intel-int3496 >> -Add documentation: Documentation/extcon/intel-int3496.txt >> -Fold usb_otg_set_port() into usb_otg_do_usb_id() >> -Unify / cleanup error messages >> --- >> Documentation/extcon/intel-int3496.txt | 22 ++++ >> drivers/extcon/Kconfig | 10 ++ >> drivers/extcon/Makefile | 1 + >> drivers/extcon/extcon-intel-int3496.c | 179 +++++++++++++++++++++++++++++++++ >> 4 files changed, 212 insertions(+) >> create mode 100644 Documentation/extcon/intel-int3496.txt >> create mode 100644 drivers/extcon/extcon-intel-int3496.c >> >> diff --git a/Documentation/extcon/intel-int3496.txt b/Documentation/extcon/intel-int3496.txt >> new file mode 100644 >> index 0000000..af0b366 >> --- /dev/null >> +++ b/Documentation/extcon/intel-int3496.txt >> @@ -0,0 +1,22 @@ >> +Intel INT3496 ACPI device extcon driver documentation >> +----------------------------------------------------- >> + >> +The Intel INT3496 ACPI device extcon driver is a driver for ACPI >> +devices with an acpi-id of INT3496, such as found for example on >> +Intel Baytrail and Cherrytrail tablets. >> + >> +This ACPI device describes how the OS can read the id-pin of the devices' >> +USB-otg port, as well as how it optionally can enable Vbus output on the >> +otg port and how it can optionally control the muxing of the data pins >> +between an USB host and an USB peripheral controller. >> + >> +The ACPI devices exposes this functionality by returning an array with up >> +to 3 gpio descriptors from its ACPI _CRS (Current Resource Settings) call: >> + >> +Index 0: The input gpio for the id-pin, this is always present and valid >> +Index 1: The output gpio for enabling Vbus output from the device to the otg >> + port, write 1 to enable the Vbus output (this gpio descriptor may >> + be absent or invalid) >> +Index 2: The output gpio for muxing of the data pins between the USB host and >> + the USB peripheral controller, write 1 to mux to the peripheral >> + controller >> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >> index 04788d9..1ed3860 100644 >> --- a/drivers/extcon/Kconfig >> +++ b/drivers/extcon/Kconfig >> @@ -35,6 +35,16 @@ config EXTCON_AXP288 >> Say Y here to enable support for USB peripheral detection >> and USB MUX switching by X-Power AXP288 PMIC. >> >> +config EXTCON_INTEL_INT3496 > > Need to reorder alphabetically. EXTCON_INTEL_INT3496 should be > positioned on below EXTCON_GPIO. > >> + tristate "Intel INT3496 ACPI device extcon driver" >> + depends on GPIOLIB && ACPI >> + help >> + Say Y here to enable extcon support for USB OTG ports controlled by >> + an Intel INT3496 ACPI device. >> + >> + This ACPI device is typically found on Intel Baytrail or Cherrytrail >> + based tablets, or other Baytrail / Cherrytrail devices. > > The upper two line used the 'space' without 'tab'. > >> + >> config EXTCON_GPIO >> tristate "GPIO extcon support" >> depends on GPIOLIB || COMPILE_TEST >> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >> index 31a0a99..237ac3f 100644 >> --- a/drivers/extcon/Makefile >> +++ b/drivers/extcon/Makefile >> @@ -8,6 +8,7 @@ obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o >> obj-$(CONFIG_EXTCON_ARIZONA) += extcon-arizona.o >> obj-$(CONFIG_EXTCON_AXP288) += extcon-axp288.o >> obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o >> +obj-$(CONFIG_EXTCON_INTEL_INT3496) += extcon-intel-int3496.o >> obj-$(CONFIG_EXTCON_MAX14577) += extcon-max14577.o >> obj-$(CONFIG_EXTCON_MAX3355) += extcon-max3355.o >> obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o >> diff --git a/drivers/extcon/extcon-intel-int3496.c b/drivers/extcon/extcon-intel-int3496.c >> new file mode 100644 >> index 0000000..a3131b0 >> --- /dev/null >> +++ b/drivers/extcon/extcon-intel-int3496.c >> @@ -0,0 +1,179 @@ >> +/* >> + * Intel INT3496 ACPI device extcon driver >> + * >> + * Copyright (c) 2016 Hans de Goede >> + * >> + * Based on android x86 kernel code which is: >> + * >> + * Copyright (c) 2014, Intel Corporation. >> + * Author: David Cohen >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define INT3496_GPIO_USB_ID 0 >> +#define INT3496_GPIO_VBUS_EN 1 >> +#define INT3496_GPIO_USB_MUX 2 >> +#define DEBOUNCE_TIME msecs_to_jiffies(50) >> + >> +struct int3496_data { >> + struct device *dev; >> + struct extcon_dev *edev; >> + struct delayed_work work; >> + struct gpio_desc *gpio_usb_id; >> + struct gpio_desc *gpio_vbus_en; >> + struct gpio_desc *gpio_usb_mux; >> + int usb_id_irq; >> +}; >> + >> +static const unsigned int int3496_cable[] = { >> + EXTCON_USB_HOST, >> + EXTCON_NONE, >> +}; >> + >> +static void int3496_do_usb_id(struct work_struct *work) >> +{ >> + struct int3496_data *data = >> + container_of(work, struct int3496_data, work.work); >> + int id = gpiod_get_value_cansleep(data->gpio_usb_id); >> + >> + /* id == 1: PERIPHERAL, id == 0: HOST */ >> + dev_dbg(data->dev, "Connected %s cable\n", id ? "PERIPHERAL" : "HOST"); >> + >> + /* >> + * Peripheral: set USB mux to peripheral and disable VBUS >> + * Host: set USB mux to host and enable VBUS >> + */ >> + if (!IS_ERR(data->gpio_usb_mux)) >> + gpiod_direction_output(data->gpio_usb_mux, id); >> + >> + if (!IS_ERR(data->gpio_vbus_en)) >> + gpiod_direction_output(data->gpio_vbus_en, !id); >> + >> + extcon_set_state_sync(data->edev, EXTCON_USB_HOST, !id); >> +} >> + >> +static irqreturn_t int3496_thread_isr(int irq, void *priv) >> +{ >> + struct int3496_data *data = priv; >> + >> + /* Let the pin settle before processing it */ >> + mod_delayed_work(system_wq, &data->work, DEBOUNCE_TIME); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int int3496_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct int3496_data *data; >> + int ret; >> + >> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->dev = dev; >> + INIT_DELAYED_WORK(&data->work, int3496_do_usb_id); >> + >> + data->gpio_usb_id = devm_gpiod_get_index(dev, "id", >> + INT3496_GPIO_USB_ID, >> + GPIOD_IN); >> + if (IS_ERR(data->gpio_usb_id)) { >> + ret = PTR_ERR(data->gpio_usb_id); >> + dev_err(dev, "can't request USB ID GPIO: %d\n", ret); >> + return ret; >> + } >> + >> + data->usb_id_irq = gpiod_to_irq(data->gpio_usb_id); >> + if (data->usb_id_irq <= 0) { >> + dev_err(dev, "can't get USB ID IRQ: %d\n", data->usb_id_irq); >> + return -EINVAL; >> + } >> + >> + data->gpio_vbus_en = devm_gpiod_get_index(dev, "vbus en", >> + INT3496_GPIO_VBUS_EN, >> + GPIOD_ASIS); >> + if (IS_ERR(data->gpio_vbus_en)) >> + dev_info(dev, "can't request VBUS EN GPIO\n"); >> + >> + data->gpio_usb_mux = devm_gpiod_get_index(dev, "usb mux", >> + INT3496_GPIO_USB_MUX, >> + GPIOD_ASIS); >> + if (IS_ERR(data->gpio_usb_mux)) >> + dev_info(dev, "can't request USB MUX GPIO\n"); >> + >> + /* register extcon device */ >> + data->edev = devm_extcon_dev_allocate(dev, int3496_cable); >> + if (IS_ERR(data->edev)) >> + return -ENOMEM; >> + >> + ret = devm_extcon_dev_register(dev, data->edev); >> + if (ret < 0) { >> + dev_err(dev, "can't register extcon device: %d\n", ret); >> + return ret; >> + } >> + >> + ret = devm_request_threaded_irq(dev, data->usb_id_irq, >> + NULL, int3496_thread_isr, >> + IRQF_SHARED | IRQF_ONESHOT | >> + IRQF_TRIGGER_RISING | >> + IRQF_TRIGGER_FALLING, >> + dev_name(dev), data); >> + if (ret < 0) { >> + dev_err(dev, "can't request IRQ for USB ID GPIO: %d\n", ret); >> + return ret; >> + } >> + >> + /* queue initial processing of id-pin */ >> + queue_delayed_work(system_wq, &data->work, 0); >> + >> + platform_set_drvdata(pdev, data); >> + >> + return 0; >> +} >> + >> +static int int3496_remove(struct platform_device *pdev) >> +{ >> + struct int3496_data *data = platform_get_drvdata(pdev); >> + >> + devm_free_irq(&pdev->dev, data->usb_id_irq, data); >> + cancel_delayed_work_sync(&data->work); >> + >> + return 0; >> +} >> + >> +static struct acpi_device_id int3496_acpi_match[] = { >> + { "INT3496" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(acpi, int3496_acpi_match); >> + >> +static struct platform_driver int3496_driver = { >> + .driver = { >> + .name = "intel-int3496", >> + .acpi_match_table = int3496_acpi_match, >> + }, >> + .probe = int3496_probe, >> + .remove = int3496_remove, >> +}; >> + >> +module_platform_driver(int3496_driver); >> + >> +MODULE_AUTHOR("Hans de Goede "); >> +MODULE_DESCRIPTION("Intel INT3496 ACPI device extcon driver"); >> +MODULE_LICENSE("GPL"); >> -- >> 2.9.3 >> > > >