* [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header @ 2019-03-18 9:52 ` Andy Shevchenko 2019-03-18 9:52 ` [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC Andy Shevchenko 2019-03-18 10:05 ` [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header Chanwoo Choi 0 siblings, 2 replies; 8+ messages in thread From: Andy Shevchenko @ 2019-03-18 9:52 UTC (permalink / raw) To: MyungJoo Ham, Chanwoo Choi, linux-kernel, Hans de Goede; +Cc: Andy Shevchenko We are going to use some definitions in the other Intel extcon drivers, thus, split out them to a common header file. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/extcon/extcon-intel-cht-wc.c | 21 +++++++-------------- drivers/extcon/extcon-intel.h | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 drivers/extcon/extcon-intel.h diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c index 5ef215297101..110bd38a4d24 100644 --- a/drivers/extcon/extcon-intel-cht-wc.c +++ b/drivers/extcon/extcon-intel-cht-wc.c @@ -17,6 +17,8 @@ #include <linux/regmap.h> #include <linux/slab.h> +#include "extcon-intel.h" + #define CHT_WC_PHYCTRL 0x5e07 #define CHT_WC_CHGRCTRL0 0x5e16 @@ -65,15 +67,6 @@ #define CHT_WC_VBUS_GPIO_CTLO_DRV_OD BIT(4) #define CHT_WC_VBUS_GPIO_CTLO_DIR_OUT BIT(5) -enum cht_wc_usb_id { - USB_ID_OTG, - USB_ID_GND, - USB_ID_FLOAT, - USB_RID_A, - USB_RID_B, - USB_RID_C, -}; - enum cht_wc_mux_select { MUX_SEL_PMIC = 0, MUX_SEL_SOC, @@ -101,9 +94,9 @@ static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts) { switch ((pwrsrc_sts & CHT_WC_PWRSRC_USBID_MASK) >> CHT_WC_PWRSRC_USBID_SHIFT) { case CHT_WC_PWRSRC_RID_GND: - return USB_ID_GND; + return INTEL_USB_ID_GND; case CHT_WC_PWRSRC_RID_FLOAT: - return USB_ID_FLOAT; + return INTEL_USB_ID_FLOAT; case CHT_WC_PWRSRC_RID_ACA: default: /* @@ -111,7 +104,7 @@ static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts) * the USBID GPADC channel here and determine ACA role * based on that. */ - return USB_ID_FLOAT; + return INTEL_USB_ID_FLOAT; } } @@ -221,7 +214,7 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext) } id = cht_wc_extcon_get_id(ext, pwrsrc_sts); - if (id == USB_ID_GND) { + if (id == INTEL_USB_ID_GND) { /* The 5v boost causes a false VBUS / SDP detect, skip */ goto charger_det_done; } @@ -248,7 +241,7 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext) ext->previous_cable = cable; } - ext->usb_host = ((id == USB_ID_GND) || (id == USB_RID_A)); + ext->usb_host = ((id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A)); extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host); } diff --git a/drivers/extcon/extcon-intel.h b/drivers/extcon/extcon-intel.h new file mode 100644 index 000000000000..455986b2b004 --- /dev/null +++ b/drivers/extcon/extcon-intel.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Header file for Intel extcon hardware + * + * Copyright (C) 2018 Intel Corporation. All rights reserved. + */ + +#ifndef __EXTCON_INTEL_H__ +#define __EXTCON_INTEL_H__ + +enum extcon_intel_usb_id { + INTEL_USB_ID_OTG, + INTEL_USB_ID_GND, + INTEL_USB_ID_FLOAT, + INTEL_USB_RID_A, + INTEL_USB_RID_B, + INTEL_USB_RID_C, +}; + +#endif /* __EXTCON_INTEL_H__ */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC 2019-03-18 9:52 ` [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header Andy Shevchenko @ 2019-03-18 9:52 ` Andy Shevchenko 2019-03-18 10:11 ` Andy Shevchenko 2019-03-18 10:05 ` [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header Chanwoo Choi 1 sibling, 1 reply; 8+ messages in thread From: Andy Shevchenko @ 2019-03-18 9:52 UTC (permalink / raw) To: MyungJoo Ham, Chanwoo Choi, linux-kernel, Hans de Goede; +Cc: Andy Shevchenko TBD Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/extcon/Kconfig | 7 + drivers/extcon/Makefile | 1 + drivers/extcon/extcon-intel-mrfld.c | 256 ++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+) create mode 100644 drivers/extcon/extcon-intel-mrfld.c diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig index 8e17149655f0..75349c6cc89e 100644 --- a/drivers/extcon/Kconfig +++ b/drivers/extcon/Kconfig @@ -60,6 +60,13 @@ config EXTCON_INTEL_CHT_WC Say Y here to enable extcon support for charger detection / control on the Intel Cherrytrail Whiskey Cove PMIC. +config EXTCON_INTEL_MRFLD + tristate "Intel MErrifield Basin Cove PMIC extcon driver" + depends on INTEL_SOC_PMIC_MRFLD + help + Say Y here to enable extcon support for charger detection / control + on the Intel Merrifiel Basin Cove PMIC. + config EXTCON_MAX14577 tristate "Maxim MAX14577/77836 EXTCON Support" depends on MFD_MAX14577 diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile index 261ce4cfe209..d3941a735df3 100644 --- a/drivers/extcon/Makefile +++ b/drivers/extcon/Makefile @@ -11,6 +11,7 @@ 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_INTEL_CHT_WC) += extcon-intel-cht-wc.o +obj-$(CONFIG_EXTCON_INTEL_MRFLD) += extcon-intel-mrfld.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-mrfld.c b/drivers/extcon/extcon-intel-mrfld.c new file mode 100644 index 000000000000..d45db4975b5f --- /dev/null +++ b/drivers/extcon/extcon-intel-mrfld.c @@ -0,0 +1,256 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Extcon driver for Basin Cove PMIC + * + * Copyright (c) 2018, Intel Corporation. + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> + */ + +#include <linux/extcon-provider.h> +#include <linux/interrupt.h> +#include <linux/mfd/intel_soc_pmic.h> +#include <linux/mfd/intel_soc_pmic_mrfld.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#include "extcon-intel.h" + +#define BCOVE_USBIDCTRL 0x19 +#define BCOVE_USBIDCTRL_ID BIT(0) +#define BCOVE_USBIDCTRL_ACA BIT(1) +#define BCOVE_USBIDCTRL_ALL (BCOVE_USBIDCTRL_ID | BCOVE_USBIDCTRL_ACA) + +#define BCOVE_USBIDSTS 0x1a +#define BCOVE_USBIDSTS_GND BIT(0) +#define BCOVE_USBIDSTS_RARBRC_MASK GENMASK(2, 1) +#define BCOVE_USBIDSTS_RARBRC_SHIFT 1 +#define BCOVE_USBIDSTS_NO_ACA 0 +#define BCOVE_USBIDSTS_R_ID_A 1 +#define BCOVE_USBIDSTS_R_ID_B 2 +#define BCOVE_USBIDSTS_R_ID_C 3 +#define BCOVE_USBIDSTS_FLOAT BIT(3) +#define BCOVE_USBIDSTS_SHORT BIT(4) + +#define BCOVE_CHGRIRQ_ALL (BCOVE_CHGRIRQ_VBUSDET | BCOVE_CHGRIRQ_DCDET | \ + BCOVE_CHGRIRQ_BATTDET | BCOVE_CHGRIRQ_USBIDDET) + +#define BCOVE_CHGRCTRL0 0x4b +#define BCOVE_CHGRCTRL0_CHGRRESET BIT(0) +#define BCOVE_CHGRCTRL0_EMRGCHREN BIT(1) +#define BCOVE_CHGRCTRL0_EXTCHRDIS BIT(2) +#define BCOVE_CHGRCTRL0_SWCONTROL BIT(3) +#define BCOVE_CHGRCTRL0_TTLCK BIT(4) +#define BCOVE_CHGRCTRL0_BIT_5 BIT(5) +#define BCOVE_CHGRCTRL0_BIT_6 BIT(6) +#define BCOVE_CHGRCTRL0_CHR_WDT_NOKICK BIT(7) + +struct mrfld_extcon_data { + struct device *dev; + struct regmap *regmap; + struct extcon_dev *edev; + unsigned int status; + unsigned int id; +}; + +static const unsigned int mrfld_extcon_cable[] = { + EXTCON_USB, + EXTCON_USB_HOST, + EXTCON_CHG_USB_SDP, + EXTCON_CHG_USB_CDP, + EXTCON_CHG_USB_DCP, + EXTCON_CHG_USB_ACA, + EXTCON_NONE, +}; + +static int mrfld_extcon_clear(struct mrfld_extcon_data *data, unsigned int reg, + unsigned int mask) +{ + return regmap_update_bits(data->regmap, reg, mask, 0x00); +} + +static int mrfld_extcon_set(struct mrfld_extcon_data *data, unsigned int reg, + unsigned int mask) +{ + return regmap_update_bits(data->regmap, reg, mask, 0xff); +} + +static int mrfld_extcon_get_id(struct mrfld_extcon_data *data) +{ + struct regmap *regmap = data->regmap; + unsigned int id; + bool ground; + int ret; + + ret = regmap_read(regmap, BCOVE_USBIDSTS, &id); + if (ret) + return ret; + + if (id & BCOVE_USBIDSTS_FLOAT) + return INTEL_USB_ID_FLOAT; + + switch ((id & BCOVE_USBIDSTS_RARBRC_MASK) >> BCOVE_USBIDSTS_RARBRC_SHIFT) { + case BCOVE_USBIDSTS_R_ID_A: + return INTEL_USB_RID_A; + case BCOVE_USBIDSTS_R_ID_B: + return INTEL_USB_RID_B; + case BCOVE_USBIDSTS_R_ID_C: + return INTEL_USB_RID_C; + } + + /* + * PMIC A0 reports USBIDSTS_GND = 1 for ID_GND, + * but PMIC B0 reports USBIDSTS_GND = 0 for ID_GND. + * Thus we must check this bit at last. + */ + ground = id & BCOVE_USBIDSTS_GND; + switch ('A' + BCOVE_MAJOR(data->id)) { + case 'A': + return ground ? INTEL_USB_ID_GND : INTEL_USB_ID_FLOAT; + case 'B': + return ground ? INTEL_USB_ID_FLOAT : INTEL_USB_ID_GND; + } + + /* Unknown or unsupported type */ + return INTEL_USB_ID_FLOAT; +} + +static int mrfld_extcon_role_detect(struct mrfld_extcon_data *data) +{ + unsigned int id; + bool usb_host; + int ret; + + ret = mrfld_extcon_get_id(data); + if (ret < 0) + return ret; + + id = ret; + + usb_host = (id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A); + extcon_set_state_sync(data->edev, EXTCON_USB_HOST, usb_host); + + return 0; +} + +static int mrfld_extcon_cable_detect(struct mrfld_extcon_data *data) +{ + struct regmap *regmap = data->regmap; + unsigned int status; + int ret; + + /* + * It seems SCU firmware clears the content of BCOVE_CHGRIRQ1 + * and makes it useless for OS. Instead we compare a previously + * stored status to the current one, provided by BCOVE_SCHGRIRQ1. + */ + ret = regmap_read(regmap, BCOVE_SCHGRIRQ1, &status); + if (ret) + return ret; + + if (!(status ^ data->status)) + return -ENODATA; + + if ((status ^ data->status) & BCOVE_CHGRIRQ_USBIDDET) + ret = mrfld_extcon_role_detect(data); + + data->status = status; + return ret; +} + +static irqreturn_t mrfld_extcon_interrupt(int irq, void *dev_id) +{ + struct mrfld_extcon_data *data = dev_id; + int ret; + + ret = mrfld_extcon_cable_detect(data); + + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); + + return ret ? IRQ_NONE: IRQ_HANDLED; +} + +static int mrfld_extcon_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct intel_soc_pmic *pmic = dev_get_drvdata(dev->parent); + struct regmap *regmap = pmic->regmap; + struct mrfld_extcon_data *data; + unsigned int id; + int irq, ret; + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->dev = dev; + data->regmap = regmap; + + data->edev = devm_extcon_dev_allocate(dev, mrfld_extcon_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, irq, NULL, mrfld_extcon_interrupt, + IRQF_ONESHOT | IRQF_SHARED, pdev->name, + data); + if (ret) + return ret; + + ret = regmap_read(regmap, BCOVE_ID, &id); + if (ret) + return ret; + + data->id = id; + + mrfld_extcon_set(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); + + /* Get initial state */ + mrfld_extcon_role_detect(data); + + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); + mrfld_extcon_clear(data, BCOVE_MCHGRIRQ1, BCOVE_CHGRIRQ_ALL); + + mrfld_extcon_set(data, BCOVE_USBIDCTRL, BCOVE_USBIDCTRL_ALL); + + platform_set_drvdata(pdev, data); + return 0; +} + +static int mrfld_extcon_remove(struct platform_device *pdev) +{ + struct mrfld_extcon_data *data = platform_get_drvdata(pdev); + + mrfld_extcon_clear(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); + return 0; +} + +static const struct platform_device_id mrfld_extcon_id_table[] = { + { .name = "mrfld_bcove_extcon" }, + {} +}; +MODULE_DEVICE_TABLE(platform, mrfld_extcon_id_table); + +static struct platform_driver mrfld_extcon_driver = { + .driver = { + .name = KBUILD_MODNAME, + }, + .probe = mrfld_extcon_probe, + .remove = mrfld_extcon_remove, + .id_table = mrfld_extcon_id_table, +}; +module_platform_driver(mrfld_extcon_driver); + +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>"); +MODULE_DESCRIPTION("Extcon driver for Basin Cove PMIC"); +MODULE_LICENSE("GPL v2"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC 2019-03-18 9:52 ` [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC Andy Shevchenko @ 2019-03-18 10:11 ` Andy Shevchenko 2019-03-18 10:38 ` Chanwoo Choi 0 siblings, 1 reply; 8+ messages in thread From: Andy Shevchenko @ 2019-03-18 10:11 UTC (permalink / raw) To: MyungJoo Ham, Chanwoo Choi, linux-kernel, Hans de Goede On Mon, Mar 18, 2019 at 12:52:25PM +0300, Andy Shevchenko wrote: > TBD Oops. I though I have written it already. I will wait for other comments today and sent a new version with commit message filled as follows: On Intel Merrifield the Basin Cove PMIC provides a feature to detect the USB connection type. This driver utilizes the feature in order to support the USB dual role detection. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/extcon/Kconfig | 7 + > drivers/extcon/Makefile | 1 + > drivers/extcon/extcon-intel-mrfld.c | 256 ++++++++++++++++++++++++++++ > 3 files changed, 264 insertions(+) > create mode 100644 drivers/extcon/extcon-intel-mrfld.c > > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig > index 8e17149655f0..75349c6cc89e 100644 > --- a/drivers/extcon/Kconfig > +++ b/drivers/extcon/Kconfig > @@ -60,6 +60,13 @@ config EXTCON_INTEL_CHT_WC > Say Y here to enable extcon support for charger detection / control > on the Intel Cherrytrail Whiskey Cove PMIC. > > +config EXTCON_INTEL_MRFLD > + tristate "Intel MErrifield Basin Cove PMIC extcon driver" ME -> Me (will be fixed) > + depends on INTEL_SOC_PMIC_MRFLD > + help > + Say Y here to enable extcon support for charger detection / control > + on the Intel Merrifiel Basin Cove PMIC. > + > config EXTCON_MAX14577 > tristate "Maxim MAX14577/77836 EXTCON Support" > depends on MFD_MAX14577 > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile > index 261ce4cfe209..d3941a735df3 100644 > --- a/drivers/extcon/Makefile > +++ b/drivers/extcon/Makefile > @@ -11,6 +11,7 @@ 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_INTEL_CHT_WC) += extcon-intel-cht-wc.o > +obj-$(CONFIG_EXTCON_INTEL_MRFLD) += extcon-intel-mrfld.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-mrfld.c b/drivers/extcon/extcon-intel-mrfld.c > new file mode 100644 > index 000000000000..d45db4975b5f > --- /dev/null > +++ b/drivers/extcon/extcon-intel-mrfld.c > @@ -0,0 +1,256 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Extcon driver for Basin Cove PMIC > + * > + * Copyright (c) 2018, Intel Corporation. 2019 I suppose :-) > + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > + */ > + > +#include <linux/extcon-provider.h> > +#include <linux/interrupt.h> > +#include <linux/mfd/intel_soc_pmic.h> > +#include <linux/mfd/intel_soc_pmic_mrfld.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#include "extcon-intel.h" > + > +#define BCOVE_USBIDCTRL 0x19 > +#define BCOVE_USBIDCTRL_ID BIT(0) > +#define BCOVE_USBIDCTRL_ACA BIT(1) > +#define BCOVE_USBIDCTRL_ALL (BCOVE_USBIDCTRL_ID | BCOVE_USBIDCTRL_ACA) > + > +#define BCOVE_USBIDSTS 0x1a > +#define BCOVE_USBIDSTS_GND BIT(0) > +#define BCOVE_USBIDSTS_RARBRC_MASK GENMASK(2, 1) > +#define BCOVE_USBIDSTS_RARBRC_SHIFT 1 > +#define BCOVE_USBIDSTS_NO_ACA 0 > +#define BCOVE_USBIDSTS_R_ID_A 1 > +#define BCOVE_USBIDSTS_R_ID_B 2 > +#define BCOVE_USBIDSTS_R_ID_C 3 > +#define BCOVE_USBIDSTS_FLOAT BIT(3) > +#define BCOVE_USBIDSTS_SHORT BIT(4) > + > +#define BCOVE_CHGRIRQ_ALL (BCOVE_CHGRIRQ_VBUSDET | BCOVE_CHGRIRQ_DCDET | \ > + BCOVE_CHGRIRQ_BATTDET | BCOVE_CHGRIRQ_USBIDDET) > + > +#define BCOVE_CHGRCTRL0 0x4b > +#define BCOVE_CHGRCTRL0_CHGRRESET BIT(0) > +#define BCOVE_CHGRCTRL0_EMRGCHREN BIT(1) > +#define BCOVE_CHGRCTRL0_EXTCHRDIS BIT(2) > +#define BCOVE_CHGRCTRL0_SWCONTROL BIT(3) > +#define BCOVE_CHGRCTRL0_TTLCK BIT(4) > +#define BCOVE_CHGRCTRL0_BIT_5 BIT(5) > +#define BCOVE_CHGRCTRL0_BIT_6 BIT(6) > +#define BCOVE_CHGRCTRL0_CHR_WDT_NOKICK BIT(7) > + > +struct mrfld_extcon_data { > + struct device *dev; > + struct regmap *regmap; > + struct extcon_dev *edev; > + unsigned int status; > + unsigned int id; > +}; > + > +static const unsigned int mrfld_extcon_cable[] = { > + EXTCON_USB, > + EXTCON_USB_HOST, > + EXTCON_CHG_USB_SDP, > + EXTCON_CHG_USB_CDP, > + EXTCON_CHG_USB_DCP, > + EXTCON_CHG_USB_ACA, > + EXTCON_NONE, > +}; > + > +static int mrfld_extcon_clear(struct mrfld_extcon_data *data, unsigned int reg, > + unsigned int mask) > +{ > + return regmap_update_bits(data->regmap, reg, mask, 0x00); > +} > + > +static int mrfld_extcon_set(struct mrfld_extcon_data *data, unsigned int reg, > + unsigned int mask) > +{ > + return regmap_update_bits(data->regmap, reg, mask, 0xff); > +} > + > +static int mrfld_extcon_get_id(struct mrfld_extcon_data *data) > +{ > + struct regmap *regmap = data->regmap; > + unsigned int id; > + bool ground; > + int ret; > + > + ret = regmap_read(regmap, BCOVE_USBIDSTS, &id); > + if (ret) > + return ret; > + > + if (id & BCOVE_USBIDSTS_FLOAT) > + return INTEL_USB_ID_FLOAT; > + > + switch ((id & BCOVE_USBIDSTS_RARBRC_MASK) >> BCOVE_USBIDSTS_RARBRC_SHIFT) { > + case BCOVE_USBIDSTS_R_ID_A: > + return INTEL_USB_RID_A; > + case BCOVE_USBIDSTS_R_ID_B: > + return INTEL_USB_RID_B; > + case BCOVE_USBIDSTS_R_ID_C: > + return INTEL_USB_RID_C; > + } > + > + /* > + * PMIC A0 reports USBIDSTS_GND = 1 for ID_GND, > + * but PMIC B0 reports USBIDSTS_GND = 0 for ID_GND. > + * Thus we must check this bit at last. > + */ > + ground = id & BCOVE_USBIDSTS_GND; > + switch ('A' + BCOVE_MAJOR(data->id)) { > + case 'A': > + return ground ? INTEL_USB_ID_GND : INTEL_USB_ID_FLOAT; > + case 'B': > + return ground ? INTEL_USB_ID_FLOAT : INTEL_USB_ID_GND; > + } > + > + /* Unknown or unsupported type */ > + return INTEL_USB_ID_FLOAT; > +} > + > +static int mrfld_extcon_role_detect(struct mrfld_extcon_data *data) > +{ > + unsigned int id; > + bool usb_host; > + int ret; > + > + ret = mrfld_extcon_get_id(data); > + if (ret < 0) > + return ret; > + > + id = ret; > + > + usb_host = (id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A); > + extcon_set_state_sync(data->edev, EXTCON_USB_HOST, usb_host); > + > + return 0; > +} > + > +static int mrfld_extcon_cable_detect(struct mrfld_extcon_data *data) > +{ > + struct regmap *regmap = data->regmap; > + unsigned int status; > + int ret; > + > + /* > + * It seems SCU firmware clears the content of BCOVE_CHGRIRQ1 > + * and makes it useless for OS. Instead we compare a previously > + * stored status to the current one, provided by BCOVE_SCHGRIRQ1. > + */ > + ret = regmap_read(regmap, BCOVE_SCHGRIRQ1, &status); > + if (ret) > + return ret; > + > + if (!(status ^ data->status)) > + return -ENODATA; > + > + if ((status ^ data->status) & BCOVE_CHGRIRQ_USBIDDET) > + ret = mrfld_extcon_role_detect(data); > + > + data->status = status; > + return ret; > +} > + > +static irqreturn_t mrfld_extcon_interrupt(int irq, void *dev_id) > +{ > + struct mrfld_extcon_data *data = dev_id; > + int ret; > + > + ret = mrfld_extcon_cable_detect(data); > + > + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); > + > + return ret ? IRQ_NONE: IRQ_HANDLED; > +} > + > +static int mrfld_extcon_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct intel_soc_pmic *pmic = dev_get_drvdata(dev->parent); > + struct regmap *regmap = pmic->regmap; > + struct mrfld_extcon_data *data; > + unsigned int id; > + int irq, ret; > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->dev = dev; > + data->regmap = regmap; > + > + data->edev = devm_extcon_dev_allocate(dev, mrfld_extcon_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, irq, NULL, mrfld_extcon_interrupt, > + IRQF_ONESHOT | IRQF_SHARED, pdev->name, > + data); > + if (ret) > + return ret; > + > + ret = regmap_read(regmap, BCOVE_ID, &id); > + if (ret) > + return ret; > + > + data->id = id; > + > + mrfld_extcon_set(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); > + > + /* Get initial state */ > + mrfld_extcon_role_detect(data); > + > + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); > + mrfld_extcon_clear(data, BCOVE_MCHGRIRQ1, BCOVE_CHGRIRQ_ALL); > + > + mrfld_extcon_set(data, BCOVE_USBIDCTRL, BCOVE_USBIDCTRL_ALL); > + > + platform_set_drvdata(pdev, data); > + return 0; > +} > + > +static int mrfld_extcon_remove(struct platform_device *pdev) > +{ > + struct mrfld_extcon_data *data = platform_get_drvdata(pdev); > + > + mrfld_extcon_clear(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); > + return 0; > +} > + > +static const struct platform_device_id mrfld_extcon_id_table[] = { > + { .name = "mrfld_bcove_extcon" }, > + {} > +}; > +MODULE_DEVICE_TABLE(platform, mrfld_extcon_id_table); > + > +static struct platform_driver mrfld_extcon_driver = { > + .driver = { > + .name = KBUILD_MODNAME, > + }, > + .probe = mrfld_extcon_probe, > + .remove = mrfld_extcon_remove, > + .id_table = mrfld_extcon_id_table, > +}; > +module_platform_driver(mrfld_extcon_driver); > + > +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>"); > +MODULE_DESCRIPTION("Extcon driver for Basin Cove PMIC"); > +MODULE_LICENSE("GPL v2"); > -- > 2.20.1 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC 2019-03-18 10:11 ` Andy Shevchenko @ 2019-03-18 10:38 ` Chanwoo Choi 2019-03-18 10:50 ` Chanwoo Choi 2019-03-18 12:46 ` Andy Shevchenko 0 siblings, 2 replies; 8+ messages in thread From: Chanwoo Choi @ 2019-03-18 10:38 UTC (permalink / raw) To: Andy Shevchenko, MyungJoo Ham, linux-kernel, Hans de Goede Hi Andy, Thanks for comment. I add my comments and then you have to rebase it on latest v5.0-rc1 because the merge conflict happen on v5.0-rc1. On 19. 3. 18. 오후 7:11, Andy Shevchenko wrote: > On Mon, Mar 18, 2019 at 12:52:25PM +0300, Andy Shevchenko wrote: >> TBD > > Oops. > I though I have written it already. > > I will wait for other comments today and sent a new version with commit message > filled as follows: > > On Intel Merrifield the Basin Cove PMIC provides a feature to detect > the USB connection type. This driver utilizes the feature in order to support > the USB dual role detection. > >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> drivers/extcon/Kconfig | 7 + >> drivers/extcon/Makefile | 1 + >> drivers/extcon/extcon-intel-mrfld.c | 256 ++++++++++++++++++++++++++++ >> 3 files changed, 264 insertions(+) >> create mode 100644 drivers/extcon/extcon-intel-mrfld.c >> >> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >> index 8e17149655f0..75349c6cc89e 100644 >> --- a/drivers/extcon/Kconfig >> +++ b/drivers/extcon/Kconfig >> @@ -60,6 +60,13 @@ config EXTCON_INTEL_CHT_WC >> Say Y here to enable extcon support for charger detection / control >> on the Intel Cherrytrail Whiskey Cove PMIC. >> >> +config EXTCON_INTEL_MRFLD > >> + tristate "Intel MErrifield Basin Cove PMIC extcon driver" > > ME -> Me (will be fixed) > >> + depends on INTEL_SOC_PMIC_MRFLD This driver uses the regmap interface. So, you better to add following dependency? - select REGMAP_I2C or REGMAP_SPI But, if 'INTEL_SOC_PMIC_MRFLE' selects already REGMAP_* configuration. It is not necessary. >> + help >> + Say Y here to enable extcon support for charger detection / control >> + on the Intel Merrifiel Basin Cove PMIC. What is correct word? - Merrifield? is used on above - Merrifiel? >> + >> config EXTCON_MAX14577 >> tristate "Maxim MAX14577/77836 EXTCON Support" >> depends on MFD_MAX14577 >> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >> index 261ce4cfe209..d3941a735df3 100644 >> --- a/drivers/extcon/Makefile >> +++ b/drivers/extcon/Makefile >> @@ -11,6 +11,7 @@ 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_INTEL_CHT_WC) += extcon-intel-cht-wc.o >> +obj-$(CONFIG_EXTCON_INTEL_MRFLD) += extcon-intel-mrfld.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-mrfld.c b/drivers/extcon/extcon-intel-mrfld.c >> new file mode 100644 >> index 000000000000..d45db4975b5f >> --- /dev/null >> +++ b/drivers/extcon/extcon-intel-mrfld.c >> @@ -0,0 +1,256 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Extcon driver for Basin Cove PMIC >> + * >> + * Copyright (c) 2018, Intel Corporation. > > 2019 I suppose :-) Right. > >> + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> + */ >> + >> +#include <linux/extcon-provider.h> >> +#include <linux/interrupt.h> >> +#include <linux/mfd/intel_soc_pmic.h> >> +#include <linux/mfd/intel_soc_pmic_mrfld.h> >> +#include <linux/mod_devicetable.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> + >> +#include "extcon-intel.h" >> + >> +#define BCOVE_USBIDCTRL 0x19 >> +#define BCOVE_USBIDCTRL_ID BIT(0) >> +#define BCOVE_USBIDCTRL_ACA BIT(1) >> +#define BCOVE_USBIDCTRL_ALL (BCOVE_USBIDCTRL_ID | BCOVE_USBIDCTRL_ACA) >> + >> +#define BCOVE_USBIDSTS 0x1a >> +#define BCOVE_USBIDSTS_GND BIT(0) >> +#define BCOVE_USBIDSTS_RARBRC_MASK GENMASK(2, 1) >> +#define BCOVE_USBIDSTS_RARBRC_SHIFT 1 >> +#define BCOVE_USBIDSTS_NO_ACA 0 >> +#define BCOVE_USBIDSTS_R_ID_A 1 >> +#define BCOVE_USBIDSTS_R_ID_B 2 >> +#define BCOVE_USBIDSTS_R_ID_C 3 >> +#define BCOVE_USBIDSTS_FLOAT BIT(3) >> +#define BCOVE_USBIDSTS_SHORT BIT(4) >> + >> +#define BCOVE_CHGRIRQ_ALL (BCOVE_CHGRIRQ_VBUSDET | BCOVE_CHGRIRQ_DCDET | \ >> + BCOVE_CHGRIRQ_BATTDET | BCOVE_CHGRIRQ_USBIDDET) >> + >> +#define BCOVE_CHGRCTRL0 0x4b >> +#define BCOVE_CHGRCTRL0_CHGRRESET BIT(0) >> +#define BCOVE_CHGRCTRL0_EMRGCHREN BIT(1) >> +#define BCOVE_CHGRCTRL0_EXTCHRDIS BIT(2) >> +#define BCOVE_CHGRCTRL0_SWCONTROL BIT(3) >> +#define BCOVE_CHGRCTRL0_TTLCK BIT(4) >> +#define BCOVE_CHGRCTRL0_BIT_5 BIT(5) >> +#define BCOVE_CHGRCTRL0_BIT_6 BIT(6) >> +#define BCOVE_CHGRCTRL0_CHR_WDT_NOKICK BIT(7) >> + >> +struct mrfld_extcon_data { >> + struct device *dev; >> + struct regmap *regmap; >> + struct extcon_dev *edev; >> + unsigned int status; >> + unsigned int id; >> +}; >> + >> +static const unsigned int mrfld_extcon_cable[] = { >> + EXTCON_USB, >> + EXTCON_USB_HOST, >> + EXTCON_CHG_USB_SDP, >> + EXTCON_CHG_USB_CDP, >> + EXTCON_CHG_USB_DCP, >> + EXTCON_CHG_USB_ACA, >> + EXTCON_NONE, >> +}; >> + >> +static int mrfld_extcon_clear(struct mrfld_extcon_data *data, unsigned int reg, >> + unsigned int mask) >> +{ >> + return regmap_update_bits(data->regmap, reg, mask, 0x00); >> +} >> + >> +static int mrfld_extcon_set(struct mrfld_extcon_data *data, unsigned int reg, >> + unsigned int mask) >> +{ >> + return regmap_update_bits(data->regmap, reg, mask, 0xff); >> +} mrfld_extcon_clear() and mrfld_extcon_set() are just wrapper function for regmap interface. I think that you better to define the meaningful defintion for '0x00' and '0xff' as following: (just example, you may make the more correct name) #define INTEL_MRFLD_RESET 0x00 #define INTEL_MRFLD_SET 0xff And then you better to use the 'regmap_update_bits()' function directly instead of mrfld_extcon_clear/set'. Also, you should handle the exception hanlding when using regmap function. >> + >> +static int mrfld_extcon_get_id(struct mrfld_extcon_data *data) >> +{ >> + struct regmap *regmap = data->regmap; >> + unsigned int id; >> + bool ground; >> + int ret; >> + >> + ret = regmap_read(regmap, BCOVE_USBIDSTS, &id); >> + if (ret) >> + return ret; >> + >> + if (id & BCOVE_USBIDSTS_FLOAT) >> + return INTEL_USB_ID_FLOAT; >> + >> + switch ((id & BCOVE_USBIDSTS_RARBRC_MASK) >> BCOVE_USBIDSTS_RARBRC_SHIFT) { >> + case BCOVE_USBIDSTS_R_ID_A: >> + return INTEL_USB_RID_A; >> + case BCOVE_USBIDSTS_R_ID_B: >> + return INTEL_USB_RID_B; >> + case BCOVE_USBIDSTS_R_ID_C: >> + return INTEL_USB_RID_C; Please add 'default' statement for exception handling. >> + } >> + >> + /* >> + * PMIC A0 reports USBIDSTS_GND = 1 for ID_GND, >> + * but PMIC B0 reports USBIDSTS_GND = 0 for ID_GND. >> + * Thus we must check this bit at last. >> + */ >> + ground = id & BCOVE_USBIDSTS_GND; >> + switch ('A' + BCOVE_MAJOR(data->id)) { >> + case 'A': >> + return ground ? INTEL_USB_ID_GND : INTEL_USB_ID_FLOAT; >> + case 'B': >> + return ground ? INTEL_USB_ID_FLOAT : INTEL_USB_ID_GND; >> + } >> + >> + /* Unknown or unsupported type */ >> + return INTEL_USB_ID_FLOAT; >> +} >> + >> +static int mrfld_extcon_role_detect(struct mrfld_extcon_data *data) >> +{ >> + unsigned int id; >> + bool usb_host; >> + int ret;>> + >> + ret = mrfld_extcon_get_id(data); >> + if (ret < 0) >> + return ret; >> + >> + id = ret; >> + >> + usb_host = (id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A); >> + extcon_set_state_sync(data->edev, EXTCON_USB_HOST, usb_host); >> + >> + return 0; >> +} >> + >> +static int mrfld_extcon_cable_detect(struct mrfld_extcon_data *data) >> +{ >> + struct regmap *regmap = data->regmap; >> + unsigned int status; >> + int ret; >> + >> + /* >> + * It seems SCU firmware clears the content of BCOVE_CHGRIRQ1 >> + * and makes it useless for OS. Instead we compare a previously >> + * stored status to the current one, provided by BCOVE_SCHGRIRQ1. >> + */ >> + ret = regmap_read(regmap, BCOVE_SCHGRIRQ1, &status); >> + if (ret) >> + return ret; >> + >> + if (!(status ^ data->status)) >> + return -ENODATA; >> + >> + if ((status ^ data->status) & BCOVE_CHGRIRQ_USBIDDET) >> + ret = mrfld_extcon_role_detect(data); This line gets the return value from mrfld_extcon_role_detect(data) without any error handling and then the below line just saves 'status' to 'data->status' regardless of 'ret' value. I think that you have to handle the error case of 'ret = mrfld_extcon_role_detect(data)'. >> + >> + data->status = status; nitpick. better to add one blank line. >> + return ret; >> +} >> + >> +static irqreturn_t mrfld_extcon_interrupt(int irq, void *dev_id) >> +{ >> + struct mrfld_extcon_data *data = dev_id; >> + int ret; >> + >> + ret = mrfld_extcon_cable_detect(data); >> + >> + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); >> + >> + return ret ? IRQ_NONE: IRQ_HANDLED; >> +} >> + >> +static int mrfld_extcon_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct intel_soc_pmic *pmic = dev_get_drvdata(dev->parent); >> + struct regmap *regmap = pmic->regmap; >> + struct mrfld_extcon_data *data; >> + unsigned int id; >> + int irq, ret; >> + >> + irq = platform_get_irq(pdev, 0); >> + if (irq < 0) >> + return irq; >> + >> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->dev = dev; >> + data->regmap = regmap; >> + >> + data->edev = devm_extcon_dev_allocate(dev, mrfld_extcon_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, irq, NULL, mrfld_extcon_interrupt, >> + IRQF_ONESHOT | IRQF_SHARED, pdev->name, >> + data); >> + if (ret) >> + return ret; You better add the error log with dev_err. >> + >> + ret = regmap_read(regmap, BCOVE_ID, &id); >> + if (ret) >> + return ret; ditto for error log. >> + >> + data->id = id; >> + >> + mrfld_extcon_set(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); >> + >> + /* Get initial state */ >> + mrfld_extcon_role_detect(data); Please handle the return value for exception handling with log. >> + >> + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); >> + mrfld_extcon_clear(data, BCOVE_MCHGRIRQ1, BCOVE_CHGRIRQ_ALL); >> + >> + mrfld_extcon_set(data, BCOVE_USBIDCTRL, BCOVE_USBIDCTRL_ALL); >> + >> + platform_set_drvdata(pdev, data); nitpick. better to add one blank line. >> + return 0; >> +} >> + >> +static int mrfld_extcon_remove(struct platform_device *pdev) >> +{ >> + struct mrfld_extcon_data *data = platform_get_drvdata(pdev); >> + >> + mrfld_extcon_clear(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); nitpick. better to add one blank line. >> + return 0; >> +} >> + >> +static const struct platform_device_id mrfld_extcon_id_table[] = { >> + { .name = "mrfld_bcove_extcon" }, I think that it is not proper to use 'extcon' word for the compatible name because 'extcon' word is linuxium. So, I recommend that you remove the 'extcon' word. Instead, you better to use new word related to h/w. >> + {} >> +}; >> +MODULE_DEVICE_TABLE(platform, mrfld_extcon_id_table); >> + >> +static struct platform_driver mrfld_extcon_driver = { >> + .driver = { >> + .name = KBUILD_MODNAME, Where is the definition of KBUILD_MODNAME? Are you missing? >> + }, >> + .probe = mrfld_extcon_probe, >> + .remove = mrfld_extcon_remove, >> + .id_table = mrfld_extcon_id_table, >> +}; >> +module_platform_driver(mrfld_extcon_driver); >> + >> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>"); >> +MODULE_DESCRIPTION("Extcon driver for Basin Cove PMIC"); Add the 'Merrifield' word in front of 'Basin Cove PMIC' as following: - Merrifield Basic Cove PMIC >> +MODULE_LICENSE("GPL v2"); >> -- >> 2.20.1 >> > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC 2019-03-18 10:38 ` Chanwoo Choi @ 2019-03-18 10:50 ` Chanwoo Choi 2019-03-18 12:46 ` Andy Shevchenko 1 sibling, 0 replies; 8+ messages in thread From: Chanwoo Choi @ 2019-03-18 10:50 UTC (permalink / raw) To: Andy Shevchenko, MyungJoo Ham, linux-kernel, Hans de Goede Hi Andy, On 19. 3. 18. 오후 7:38, Chanwoo Choi wrote: > Hi Andy, > > Thanks for comment. I add my comments > and then you have to rebase it on latest v5.0-rc1 > because the merge conflict happen on v5.0-rc1. Please rebase the extcon-next branch instead of v5.0-rc1. > > On 19. 3. 18. 오후 7:11, Andy Shevchenko wrote: >> On Mon, Mar 18, 2019 at 12:52:25PM +0300, Andy Shevchenko wrote: >>> TBD >> >> Oops. >> I though I have written it already. >> >> I will wait for other comments today and sent a new version with commit message >> filled as follows: >> >> On Intel Merrifield the Basin Cove PMIC provides a feature to detect >> the USB connection type. This driver utilizes the feature in order to support >> the USB dual role detection. >> >>> >>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> --- >>> drivers/extcon/Kconfig | 7 + >>> drivers/extcon/Makefile | 1 + >>> drivers/extcon/extcon-intel-mrfld.c | 256 ++++++++++++++++++++++++++++ >>> 3 files changed, 264 insertions(+) >>> create mode 100644 drivers/extcon/extcon-intel-mrfld.c >>> >>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>> index 8e17149655f0..75349c6cc89e 100644 >>> --- a/drivers/extcon/Kconfig >>> +++ b/drivers/extcon/Kconfig >>> @@ -60,6 +60,13 @@ config EXTCON_INTEL_CHT_WC >>> Say Y here to enable extcon support for charger detection / control >>> on the Intel Cherrytrail Whiskey Cove PMIC. >>> >>> +config EXTCON_INTEL_MRFLD >> >>> + tristate "Intel MErrifield Basin Cove PMIC extcon driver" >> >> ME -> Me (will be fixed) >> >>> + depends on INTEL_SOC_PMIC_MRFLD > > This driver uses the regmap interface. So, you better to add > following dependency? > - select REGMAP_I2C or REGMAP_SPI > > But, if 'INTEL_SOC_PMIC_MRFLE' selects already REGMAP_* > configuration. It is not necessary. > >>> + help >>> + Say Y here to enable extcon support for charger detection / control >>> + on the Intel Merrifiel Basin Cove PMIC. > > What is correct word? > - Merrifield? is used on above > - Merrifiel? > > >>> + >>> config EXTCON_MAX14577 >>> tristate "Maxim MAX14577/77836 EXTCON Support" >>> depends on MFD_MAX14577 >>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile >>> index 261ce4cfe209..d3941a735df3 100644 >>> --- a/drivers/extcon/Makefile >>> +++ b/drivers/extcon/Makefile >>> @@ -11,6 +11,7 @@ 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_INTEL_CHT_WC) += extcon-intel-cht-wc.o >>> +obj-$(CONFIG_EXTCON_INTEL_MRFLD) += extcon-intel-mrfld.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-mrfld.c b/drivers/extcon/extcon-intel-mrfld.c >>> new file mode 100644 >>> index 000000000000..d45db4975b5f >>> --- /dev/null >>> +++ b/drivers/extcon/extcon-intel-mrfld.c >>> @@ -0,0 +1,256 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Extcon driver for Basin Cove PMIC >>> + * >>> + * Copyright (c) 2018, Intel Corporation. >> >> 2019 I suppose :-) > > Right. > >> >>> + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> + */ >>> + >>> +#include <linux/extcon-provider.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/mfd/intel_soc_pmic.h> >>> +#include <linux/mfd/intel_soc_pmic_mrfld.h> >>> +#include <linux/mod_devicetable.h> >>> +#include <linux/module.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/regmap.h> >>> + >>> +#include "extcon-intel.h" >>> + >>> +#define BCOVE_USBIDCTRL 0x19 >>> +#define BCOVE_USBIDCTRL_ID BIT(0) >>> +#define BCOVE_USBIDCTRL_ACA BIT(1) >>> +#define BCOVE_USBIDCTRL_ALL (BCOVE_USBIDCTRL_ID | BCOVE_USBIDCTRL_ACA) >>> + >>> +#define BCOVE_USBIDSTS 0x1a >>> +#define BCOVE_USBIDSTS_GND BIT(0) >>> +#define BCOVE_USBIDSTS_RARBRC_MASK GENMASK(2, 1) >>> +#define BCOVE_USBIDSTS_RARBRC_SHIFT 1 >>> +#define BCOVE_USBIDSTS_NO_ACA 0 >>> +#define BCOVE_USBIDSTS_R_ID_A 1 >>> +#define BCOVE_USBIDSTS_R_ID_B 2 >>> +#define BCOVE_USBIDSTS_R_ID_C 3 >>> +#define BCOVE_USBIDSTS_FLOAT BIT(3) >>> +#define BCOVE_USBIDSTS_SHORT BIT(4) >>> + >>> +#define BCOVE_CHGRIRQ_ALL (BCOVE_CHGRIRQ_VBUSDET | BCOVE_CHGRIRQ_DCDET | \ >>> + BCOVE_CHGRIRQ_BATTDET | BCOVE_CHGRIRQ_USBIDDET) >>> + >>> +#define BCOVE_CHGRCTRL0 0x4b >>> +#define BCOVE_CHGRCTRL0_CHGRRESET BIT(0) >>> +#define BCOVE_CHGRCTRL0_EMRGCHREN BIT(1) >>> +#define BCOVE_CHGRCTRL0_EXTCHRDIS BIT(2) >>> +#define BCOVE_CHGRCTRL0_SWCONTROL BIT(3) >>> +#define BCOVE_CHGRCTRL0_TTLCK BIT(4) >>> +#define BCOVE_CHGRCTRL0_BIT_5 BIT(5) >>> +#define BCOVE_CHGRCTRL0_BIT_6 BIT(6) >>> +#define BCOVE_CHGRCTRL0_CHR_WDT_NOKICK BIT(7) >>> + >>> +struct mrfld_extcon_data { >>> + struct device *dev; >>> + struct regmap *regmap; >>> + struct extcon_dev *edev; >>> + unsigned int status; >>> + unsigned int id; >>> +}; >>> + >>> +static const unsigned int mrfld_extcon_cable[] = { >>> + EXTCON_USB, >>> + EXTCON_USB_HOST, >>> + EXTCON_CHG_USB_SDP, >>> + EXTCON_CHG_USB_CDP, >>> + EXTCON_CHG_USB_DCP, >>> + EXTCON_CHG_USB_ACA, >>> + EXTCON_NONE, >>> +}; >>> + >>> +static int mrfld_extcon_clear(struct mrfld_extcon_data *data, unsigned int reg, >>> + unsigned int mask) >>> +{ >>> + return regmap_update_bits(data->regmap, reg, mask, 0x00); >>> +} >>> + >>> +static int mrfld_extcon_set(struct mrfld_extcon_data *data, unsigned int reg, >>> + unsigned int mask) >>> +{ >>> + return regmap_update_bits(data->regmap, reg, mask, 0xff); >>> +} > > mrfld_extcon_clear() and mrfld_extcon_set() are just wrapper function > for regmap interface. I think that you better to define > the meaningful defintion for '0x00' and '0xff' as following: > > (just example, you may make the more correct name) > #define INTEL_MRFLD_RESET 0x00 > #define INTEL_MRFLD_SET 0xff > > And then you better to use the 'regmap_update_bits()' function > directly instead of mrfld_extcon_clear/set'. > > Also, you should handle the exception hanlding when using regmap function. > >>> + >>> +static int mrfld_extcon_get_id(struct mrfld_extcon_data *data) >>> +{ >>> + struct regmap *regmap = data->regmap; >>> + unsigned int id; >>> + bool ground; >>> + int ret; >>> + >>> + ret = regmap_read(regmap, BCOVE_USBIDSTS, &id); >>> + if (ret) >>> + return ret; >>> + >>> + if (id & BCOVE_USBIDSTS_FLOAT) >>> + return INTEL_USB_ID_FLOAT; >>> + >>> + switch ((id & BCOVE_USBIDSTS_RARBRC_MASK) >> BCOVE_USBIDSTS_RARBRC_SHIFT) { >>> + case BCOVE_USBIDSTS_R_ID_A: >>> + return INTEL_USB_RID_A; >>> + case BCOVE_USBIDSTS_R_ID_B: >>> + return INTEL_USB_RID_B; >>> + case BCOVE_USBIDSTS_R_ID_C: >>> + return INTEL_USB_RID_C; > > Please add 'default' statement for exception handling. > >>> + } >>> + >>> + /* >>> + * PMIC A0 reports USBIDSTS_GND = 1 for ID_GND, >>> + * but PMIC B0 reports USBIDSTS_GND = 0 for ID_GND. >>> + * Thus we must check this bit at last. >>> + */ >>> + ground = id & BCOVE_USBIDSTS_GND; >>> + switch ('A' + BCOVE_MAJOR(data->id)) { >>> + case 'A': >>> + return ground ? INTEL_USB_ID_GND : INTEL_USB_ID_FLOAT; >>> + case 'B': >>> + return ground ? INTEL_USB_ID_FLOAT : INTEL_USB_ID_GND; >>> + } >>> + >>> + /* Unknown or unsupported type */ >>> + return INTEL_USB_ID_FLOAT; >>> +} >>> + >>> +static int mrfld_extcon_role_detect(struct mrfld_extcon_data *data) >>> +{ >>> + unsigned int id; >>> + bool usb_host; >>> + int ret;>> + >>> + ret = mrfld_extcon_get_id(data); >>> + if (ret < 0) >>> + return ret; >>> + >>> + id = ret; >>> + >>> + usb_host = (id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A); >>> + extcon_set_state_sync(data->edev, EXTCON_USB_HOST, usb_host); >>> + >>> + return 0; >>> +} >>> + >>> +static int mrfld_extcon_cable_detect(struct mrfld_extcon_data *data) >>> +{ >>> + struct regmap *regmap = data->regmap; >>> + unsigned int status; >>> + int ret; >>> + >>> + /* >>> + * It seems SCU firmware clears the content of BCOVE_CHGRIRQ1 >>> + * and makes it useless for OS. Instead we compare a previously >>> + * stored status to the current one, provided by BCOVE_SCHGRIRQ1. >>> + */ >>> + ret = regmap_read(regmap, BCOVE_SCHGRIRQ1, &status); >>> + if (ret) >>> + return ret; >>> + >>> + if (!(status ^ data->status)) >>> + return -ENODATA; >>> + >>> + if ((status ^ data->status) & BCOVE_CHGRIRQ_USBIDDET) >>> + ret = mrfld_extcon_role_detect(data); > This line gets the return value from mrfld_extcon_role_detect(data) > without any error handling and then the below line just saves 'status' > to 'data->status' regardless of 'ret' value. > > I think that you have to handle the error case of > 'ret = mrfld_extcon_role_detect(data)'. > >>> + >>> + data->status = status; > > nitpick. better to add one blank line. > >>> + return ret; >>> +} >>> + >>> +static irqreturn_t mrfld_extcon_interrupt(int irq, void *dev_id) >>> +{ >>> + struct mrfld_extcon_data *data = dev_id; >>> + int ret; >>> + >>> + ret = mrfld_extcon_cable_detect(data); >>> + >>> + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); >>> + >>> + return ret ? IRQ_NONE: IRQ_HANDLED; >>> +} >>> + >>> +static int mrfld_extcon_probe(struct platform_device *pdev) >>> +{ >>> + struct device *dev = &pdev->dev; >>> + struct intel_soc_pmic *pmic = dev_get_drvdata(dev->parent); >>> + struct regmap *regmap = pmic->regmap; >>> + struct mrfld_extcon_data *data; >>> + unsigned int id; >>> + int irq, ret; >>> + >>> + irq = platform_get_irq(pdev, 0); >>> + if (irq < 0) >>> + return irq; >>> + >>> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); >>> + if (!data) >>> + return -ENOMEM; >>> + >>> + data->dev = dev; >>> + data->regmap = regmap; >>> + >>> + data->edev = devm_extcon_dev_allocate(dev, mrfld_extcon_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, irq, NULL, mrfld_extcon_interrupt, >>> + IRQF_ONESHOT | IRQF_SHARED, pdev->name, >>> + data); >>> + if (ret) >>> + return ret; > > You better add the error log with dev_err. > >>> + >>> + ret = regmap_read(regmap, BCOVE_ID, &id); >>> + if (ret) >>> + return ret; > > ditto for error log. > >>> + >>> + data->id = id; >>> + >>> + mrfld_extcon_set(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); >>> + >>> + /* Get initial state */ >>> + mrfld_extcon_role_detect(data); > > Please handle the return value for exception handling with log. > >>> + >>> + mrfld_extcon_clear(data, BCOVE_MIRQLVL1, BCOVE_LVL1_CHGR); >>> + mrfld_extcon_clear(data, BCOVE_MCHGRIRQ1, BCOVE_CHGRIRQ_ALL); >>> + >>> + mrfld_extcon_set(data, BCOVE_USBIDCTRL, BCOVE_USBIDCTRL_ALL); >>> + >>> + platform_set_drvdata(pdev, data); > > nitpick. better to add one blank line. > >>> + return 0; >>> +} >>> + >>> +static int mrfld_extcon_remove(struct platform_device *pdev) >>> +{ >>> + struct mrfld_extcon_data *data = platform_get_drvdata(pdev); >>> + >>> + mrfld_extcon_clear(data, BCOVE_CHGRCTRL0, BCOVE_CHGRCTRL0_SWCONTROL); > > nitpick. better to add one blank line. > >>> + return 0; >>> +} >>> + >>> +static const struct platform_device_id mrfld_extcon_id_table[] = { >>> + { .name = "mrfld_bcove_extcon" }, > > I think that it is not proper to use 'extcon' word for the compatible name > because 'extcon' word is linuxium. So, I recommend that you remove > the 'extcon' word. Instead, you better to use new word related to h/w. > >>> + {} >>> +}; >>> +MODULE_DEVICE_TABLE(platform, mrfld_extcon_id_table); >>> + >>> +static struct platform_driver mrfld_extcon_driver = { >>> + .driver = { >>> + .name = KBUILD_MODNAME, > > Where is the definition of KBUILD_MODNAME? Are you missing? > >>> + }, >>> + .probe = mrfld_extcon_probe, >>> + .remove = mrfld_extcon_remove, >>> + .id_table = mrfld_extcon_id_table, >>> +}; >>> +module_platform_driver(mrfld_extcon_driver); >>> + >>> +MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>"); >>> +MODULE_DESCRIPTION("Extcon driver for Basin Cove PMIC"); > > Add the 'Merrifield' word in front of 'Basin Cove PMIC' as following: > - Merrifield Basic Cove PMIC > >>> +MODULE_LICENSE("GPL v2"); >>> -- >>> 2.20.1 >>> >> > > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC 2019-03-18 10:38 ` Chanwoo Choi 2019-03-18 10:50 ` Chanwoo Choi @ 2019-03-18 12:46 ` Andy Shevchenko 2019-03-19 0:45 ` Chanwoo Choi 1 sibling, 1 reply; 8+ messages in thread From: Andy Shevchenko @ 2019-03-18 12:46 UTC (permalink / raw) To: Chanwoo Choi; +Cc: MyungJoo Ham, linux-kernel, Hans de Goede On Mon, Mar 18, 2019 at 07:38:26PM +0900, Chanwoo Choi wrote: > Thanks for comment. I add my comments > and then you have to rebase it on latest v5.0-rc1 > because the merge conflict happen on v5.0-rc1. Thanks for review, see my answers below. Non-answered items will be fixed accordingly. > >> +config EXTCON_INTEL_MRFLD > > > >> + tristate "Intel MErrifield Basin Cove PMIC extcon driver" > > > > ME -> Me (will be fixed) > > > >> + depends on INTEL_SOC_PMIC_MRFLD > > This driver uses the regmap interface. So, you better to add > following dependency? > - select REGMAP_I2C or REGMAP_SPI None of them fits this or MFD driver. See below. > But, if 'INTEL_SOC_PMIC_MRFLE' selects already REGMAP_* > configuration. It is not necessary. https://lore.kernel.org/lkml/20190318095316.69278-1-andriy.shevchenko@linux.intel.com/ It selects REGMAP_IRQ which selects necessary bits from regmap API. > >> + help > >> + Say Y here to enable extcon support for charger detection / control > >> + on the Intel Merrifiel Basin Cove PMIC. > > What is correct word? > - Merrifield? is used on above > - Merrifiel? Merrifield is a correct one. Thanks for spotting this. > >> +static int mrfld_extcon_set(struct mrfld_extcon_data *data, unsigned int reg, > >> + unsigned int mask) > >> +{ > >> + return regmap_update_bits(data->regmap, reg, mask, 0xff); > >> +} > > mrfld_extcon_clear() and mrfld_extcon_set() are just wrapper function > for regmap interface. I think that you better to define > the meaningful defintion for '0x00' and '0xff' as following: > > (just example, you may make the more correct name) > #define INTEL_MRFLD_RESET 0x00 > #define INTEL_MRFLD_SET 0xff It makes a little sense here, the idea is to reduce parameters. I could ideally write (..., mask, ~mask) for clear and (..., mask, mask) for set > And then you better to use the 'regmap_update_bits()' function > directly instead of mrfld_extcon_clear/set'. It will bring duplication of long definitions and reduce readability of the code. > >> + /* > >> + * It seems SCU firmware clears the content of BCOVE_CHGRIRQ1 > >> + * and makes it useless for OS. Instead we compare a previously > >> + * stored status to the current one, provided by BCOVE_SCHGRIRQ1. > >> + */ > >> + ret = regmap_read(regmap, BCOVE_SCHGRIRQ1, &status); > >> + if (ret) > >> + return ret; > >> + > >> + if (!(status ^ data->status)) > >> + return -ENODATA; > >> + > >> + if ((status ^ data->status) & BCOVE_CHGRIRQ_USBIDDET) > >> + ret = mrfld_extcon_role_detect(data); > This line gets the return value from mrfld_extcon_role_detect(data) > without any error handling and then the below line just saves 'status' > to 'data->status' regardless of 'ret' value. > > I think that you have to handle the error case of > 'ret = mrfld_extcon_role_detect(data)'. I'm not sure of the consequences of such change. I will give it some tests, and then will proceed accordingly. > >> + .name = KBUILD_MODNAME, > > Where is the definition of KBUILD_MODNAME? Are you missing? In the Makefile. Nothing is missed here. But I could put its content explicitly here. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC 2019-03-18 12:46 ` Andy Shevchenko @ 2019-03-19 0:45 ` Chanwoo Choi 0 siblings, 0 replies; 8+ messages in thread From: Chanwoo Choi @ 2019-03-19 0:45 UTC (permalink / raw) To: Andy Shevchenko; +Cc: MyungJoo Ham, linux-kernel, Hans de Goede Hi Andy, On 19. 3. 18. 오후 9:46, Andy Shevchenko wrote: > On Mon, Mar 18, 2019 at 07:38:26PM +0900, Chanwoo Choi wrote: > >> Thanks for comment. I add my comments >> and then you have to rebase it on latest v5.0-rc1 >> because the merge conflict happen on v5.0-rc1. > > Thanks for review, see my answers below. > Non-answered items will be fixed accordingly. > >>>> +config EXTCON_INTEL_MRFLD >>> >>>> + tristate "Intel MErrifield Basin Cove PMIC extcon driver" >>> >>> ME -> Me (will be fixed) >>> >>>> + depends on INTEL_SOC_PMIC_MRFLD >> >> This driver uses the regmap interface. So, you better to add >> following dependency? > >> - select REGMAP_I2C or REGMAP_SPI > > None of them fits this or MFD driver. See below. > >> But, if 'INTEL_SOC_PMIC_MRFLE' selects already REGMAP_* >> configuration. It is not necessary. > > https://lore.kernel.org/lkml/20190318095316.69278-1-andriy.shevchenko@linux.intel.com/ > > It selects REGMAP_IRQ which selects necessary bits from regmap API. OK. > >>>> + help >>>> + Say Y here to enable extcon support for charger detection / control >>>> + on the Intel Merrifiel Basin Cove PMIC. >> >> What is correct word? >> - Merrifield? is used on above >> - Merrifiel? > > Merrifield is a correct one. Thanks for spotting this. > >>>> +static int mrfld_extcon_set(struct mrfld_extcon_data *data, unsigned int reg, >>>> + unsigned int mask) >>>> +{ >>>> + return regmap_update_bits(data->regmap, reg, mask, 0xff); >>>> +} >> >> mrfld_extcon_clear() and mrfld_extcon_set() are just wrapper function >> for regmap interface. I think that you better to define >> the meaningful defintion for '0x00' and '0xff' as following: >> >> (just example, you may make the more correct name) >> #define INTEL_MRFLD_RESET 0x00 >> #define INTEL_MRFLD_SET 0xff > > It makes a little sense here, the idea is to reduce parameters. > > I could ideally write > (..., mask, ~mask) for clear > and > (..., mask, mask) for set > >> And then you better to use the 'regmap_update_bits()' function >> directly instead of mrfld_extcon_clear/set'. > > It will bring duplication of long definitions and reduce readability of the > code. Actually, it is not critical issue. If you don't agree my comments, you keep your style on next version. I have no any strong objection. > >>>> + /* >>>> + * It seems SCU firmware clears the content of BCOVE_CHGRIRQ1 >>>> + * and makes it useless for OS. Instead we compare a previously >>>> + * stored status to the current one, provided by BCOVE_SCHGRIRQ1. >>>> + */ >>>> + ret = regmap_read(regmap, BCOVE_SCHGRIRQ1, &status); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (!(status ^ data->status)) >>>> + return -ENODATA; >>>> + >>>> + if ((status ^ data->status) & BCOVE_CHGRIRQ_USBIDDET) >>>> + ret = mrfld_extcon_role_detect(data); >> This line gets the return value from mrfld_extcon_role_detect(data) >> without any error handling and then the below line just saves 'status' >> to 'data->status' regardless of 'ret' value. >> >> I think that you have to handle the error case of >> 'ret = mrfld_extcon_role_detect(data)'. > > I'm not sure of the consequences of such change. > I will give it some tests, and then will proceed accordingly. OK. Thanks. > >>>> + .name = KBUILD_MODNAME, >> >> Where is the definition of KBUILD_MODNAME? Are you missing? > > In the Makefile. > Nothing is missed here. > > But I could put its content explicitly here. OK. Thanks. > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header 2019-03-18 9:52 ` [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header Andy Shevchenko 2019-03-18 9:52 ` [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC Andy Shevchenko @ 2019-03-18 10:05 ` Chanwoo Choi 1 sibling, 0 replies; 8+ messages in thread From: Chanwoo Choi @ 2019-03-18 10:05 UTC (permalink / raw) To: Andy Shevchenko, MyungJoo Ham, linux-kernel, Hans de Goede Hi, Looks good to me. But just there is minor one comment as following: - 2018 -> 2019 On 19. 3. 18. 오후 6:52, Andy Shevchenko wrote: > We are going to use some definitions in the other Intel extcon drivers, > thus, split out them to a common header file. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/extcon/extcon-intel-cht-wc.c | 21 +++++++-------------- > drivers/extcon/extcon-intel.h | 20 ++++++++++++++++++++ > 2 files changed, 27 insertions(+), 14 deletions(-) > create mode 100644 drivers/extcon/extcon-intel.h > > diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c > index 5ef215297101..110bd38a4d24 100644 > --- a/drivers/extcon/extcon-intel-cht-wc.c > +++ b/drivers/extcon/extcon-intel-cht-wc.c > @@ -17,6 +17,8 @@ > #include <linux/regmap.h> > #include <linux/slab.h> > > +#include "extcon-intel.h" > + > #define CHT_WC_PHYCTRL 0x5e07 > > #define CHT_WC_CHGRCTRL0 0x5e16 > @@ -65,15 +67,6 @@ > #define CHT_WC_VBUS_GPIO_CTLO_DRV_OD BIT(4) > #define CHT_WC_VBUS_GPIO_CTLO_DIR_OUT BIT(5) > > -enum cht_wc_usb_id { > - USB_ID_OTG, > - USB_ID_GND, > - USB_ID_FLOAT, > - USB_RID_A, > - USB_RID_B, > - USB_RID_C, > -}; > - > enum cht_wc_mux_select { > MUX_SEL_PMIC = 0, > MUX_SEL_SOC, > @@ -101,9 +94,9 @@ static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts) > { > switch ((pwrsrc_sts & CHT_WC_PWRSRC_USBID_MASK) >> CHT_WC_PWRSRC_USBID_SHIFT) { > case CHT_WC_PWRSRC_RID_GND: > - return USB_ID_GND; > + return INTEL_USB_ID_GND; > case CHT_WC_PWRSRC_RID_FLOAT: > - return USB_ID_FLOAT; > + return INTEL_USB_ID_FLOAT; > case CHT_WC_PWRSRC_RID_ACA: > default: > /* > @@ -111,7 +104,7 @@ static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts) > * the USBID GPADC channel here and determine ACA role > * based on that. > */ > - return USB_ID_FLOAT; > + return INTEL_USB_ID_FLOAT; > } > } > > @@ -221,7 +214,7 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext) > } > > id = cht_wc_extcon_get_id(ext, pwrsrc_sts); > - if (id == USB_ID_GND) { > + if (id == INTEL_USB_ID_GND) { > /* The 5v boost causes a false VBUS / SDP detect, skip */ > goto charger_det_done; > } > @@ -248,7 +241,7 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext) > ext->previous_cable = cable; > } > > - ext->usb_host = ((id == USB_ID_GND) || (id == USB_RID_A)); > + ext->usb_host = ((id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A)); > extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host); > } > > diff --git a/drivers/extcon/extcon-intel.h b/drivers/extcon/extcon-intel.h > new file mode 100644 > index 000000000000..455986b2b004 > --- /dev/null > +++ b/drivers/extcon/extcon-intel.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Header file for Intel extcon hardware > + * > + * Copyright (C) 2018 Intel Corporation. All rights reserved. 2018 -> 2019 > + */ > + > +#ifndef __EXTCON_INTEL_H__ > +#define __EXTCON_INTEL_H__ > + > +enum extcon_intel_usb_id { > + INTEL_USB_ID_OTG, > + INTEL_USB_ID_GND, > + INTEL_USB_ID_FLOAT, > + INTEL_USB_RID_A, > + INTEL_USB_RID_B, > + INTEL_USB_RID_C, > +}; > + > +#endif /* __EXTCON_INTEL_H__ */ > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-19 0:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20190318095232epcas5p27d6bafb732b79cf76d84f0de0fccda6c@epcas5p2.samsung.com>
2019-03-18 9:52 ` [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header Andy Shevchenko
2019-03-18 9:52 ` [PATCH v1 2/2] extcon: mrfld: Introduce extcon driver for Basin Cove PMIC Andy Shevchenko
2019-03-18 10:11 ` Andy Shevchenko
2019-03-18 10:38 ` Chanwoo Choi
2019-03-18 10:50 ` Chanwoo Choi
2019-03-18 12:46 ` Andy Shevchenko
2019-03-19 0:45 ` Chanwoo Choi
2019-03-18 10:05 ` [PATCH v1 1/2] extcon: intel: Split out some definitions to a common header Chanwoo Choi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).