| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 27 Apr 2015 10:44:44 +0000
From: "Pallala, Ramakrishna" <ramakrishna.pallala@...el.com>
To: Chanwoo Choi <cw00.choi@...sung.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Lee Jones <lee.jones@...aro.org>,
Samuel Ortiz <sameo@...ux.intel.com>,
Carlo Caione <carlo@...one.org>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: RE: [PATCH v3 2/2] extcon-axp288: Add axp288 extcon driver support
Hi Choi,
Please find my response below.
> On 04/09/2015 02:12 AM, Ramakrishna Pallala wrote:
> > This patch adds the extcon support for AXP288 PMIC which has the BC1.2
> > charger detection capability. Additionally it also adds the USB mux
> > switching support b/w SOC and PMIC based on GPIO control.
> >
> > Signed-off-by: Ramakrishna Pallala <ramakrishna.pallala@...el.com>
> > ---
> > drivers/extcon/Kconfig | 7 +
> > drivers/extcon/Makefile | 1 +
> > drivers/extcon/extcon-axp288.c | 462
> ++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/axp20x.h | 5 +
> > 4 files changed, 475 insertions(+)
> > create mode 100644 drivers/extcon/extcon-axp288.c
> >
> > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig index
> > 6a1f7de..f6e8b2a 100644
> > --- a/drivers/extcon/Kconfig
> > +++ b/drivers/extcon/Kconfig
> > @@ -28,6 +28,13 @@ config EXTCON_ARIZONA
> > with Wolfson Arizona devices. These are audio CODECs with
> > advanced audio accessory detection support.
> >
> > +config EXTCON_AXP288
> > + tristate "X-Power AXP288 EXTCON support"
> > + depends on MFD_AXP20X && USB_PHY
> > + help
> > + Say Y here to enable support for USB peripheral detection
> > + and USB MUX switching by X-Power AXP288 PMIC.
> > +
> > config EXTCON_GPIO
> > tristate "GPIO extcon support"
> > depends on GPIOLIB
> > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile index
> > 0370b42..5429377 100644
> > --- a/drivers/extcon/Makefile
> > +++ b/drivers/extcon/Makefile
> > @@ -5,6 +5,7 @@
> > obj-$(CONFIG_EXTCON) += extcon-class.o
> > 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_MAX14577) += extcon-max14577.o
> > obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o
> > diff --git a/drivers/extcon/extcon-axp288.c
> > b/drivers/extcon/extcon-axp288.c new file mode 100644 index
> > 0000000..363552c
> > --- /dev/null
> > +++ b/drivers/extcon/extcon-axp288.c
> > @@ -0,0 +1,462 @@
> > +/*
> > + * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection
> > +driver
> > + *
> > + * Copyright (C) 2015 Intel Corporation
> > + * Author: Ramakrishna Pallala <ramakrishna.pallala@...el.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * 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 <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/io.h>
> > +#include <linux/slab.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/usb/phy.h>
> > +#include <linux/notifier.h>
> > +#include <linux/extcon.h>
> > +#include <linux/regmap.h>
> > +#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/mfd/axp20x.h>
> > +
> > +/* Power source status register */
> > +#define PS_STAT_VBUS_TRIGGER BIT(0)
> > +#define PS_STAT_BAT_CHRG_DIR BIT(2)
> > +#define PS_STAT_VBUS_ABOVE_VHOLD BIT(3)
> > +#define PS_STAT_VBUS_VALID BIT(4)
> > +#define PS_STAT_VBUS_PRESENT BIT(5)
> > +
> > +/* BC module global register */
> > +#define BC_GLOBAL_RUN BIT(0)
> > +#define BC_GLOBAL_DET_STAT BIT(2)
> > +#define BC_GLOBAL_DBP_TOUT BIT(3)
> > +#define BC_GLOBAL_VLGC_COM_SEL BIT(4)
> > +#define BC_GLOBAL_DCD_TOUT_MASK (BIT(6)|BIT(5))
> > +#define BC_GLOBAL_DCD_TOUT_300MS 0
> > +#define BC_GLOBAL_DCD_TOUT_100MS 1
> > +#define BC_GLOBAL_DCD_TOUT_500MS 2
> > +#define BC_GLOBAL_DCD_TOUT_900MS 3
> > +#define BC_GLOBAL_DCD_DET_SEL BIT(7)
> > +
> > +/* BC module vbus control and status register */
> > +#define VBUS_CNTL_DPDM_PD_EN BIT(4)
> > +#define VBUS_CNTL_DPDM_FD_EN BIT(5)
> > +#define VBUS_CNTL_FIRST_PO_STAT BIT(6)
> > +
> > +/* BC USB status register */
> > +#define USB_STAT_BUS_STAT_MASK (BIT(3)|BIT(2)|BIT(1)|BIT(0))
> > +#define USB_STAT_BUS_STAT_SHIFT 0
> > +#define USB_STAT_BUS_STAT_ATHD 0
> > +#define USB_STAT_BUS_STAT_CONN 1
> > +#define USB_STAT_BUS_STAT_SUSP 2
> > +#define USB_STAT_BUS_STAT_CONF 3
> > +#define USB_STAT_USB_SS_MODE BIT(4)
> > +#define USB_STAT_DEAD_BAT_DET BIT(6)
> > +#define USB_STAT_DBP_UNCFG BIT(7)
> > +
> > +/* BC detect status register */
> > +#define DET_STAT_MASK (BIT(7)|BIT(6)|BIT(5))
> > +#define DET_STAT_SHIFT 5
> > +#define DET_STAT_SDP 1
> > +#define DET_STAT_CDP 2
> > +#define DET_STAT_DCP 3
> > +
> > +/* IRQ enable-1 register */
> > +#define PWRSRC_IRQ_CFG_MASK (BIT(4)|BIT(3)|BIT(2))
> > +
> > +/* IRQ enable-6 register */
> > +#define BC12_IRQ_CFG_MASK BIT(1)
> > +
> > +enum axp288_extcon_reg {
> > + AXP288_PS_STAT_REG = 0x00,
> > + AXP288_PS_BOOT_REASON_REG = 0x02,
> > + AXP288_BC_GLOBAL_REG = 0x2c,
> > + AXP288_BC_VBUS_CNTL_REG = 0x2d,
> > + AXP288_BC_USB_STAT_REG = 0x2e,
> > + AXP288_BC_DET_STAT_REG = 0x2f,
> > + AXP288_PWRSRC_IRQ_CFG_REG = 0x40,
> > + AXP288_BC12_IRQ_CFG_REG = 0x45,
> > +};
> > +
> > +#define AXP288_DRV_NAME "axp288_extcon"
>
> Use the '-' instead of '_'
> - axp288_extcon -> axp288-extcon
All axp20x pmic mfd cell names are starting with "axp288_*" this one comment I got from Lee Jones on mfd patch.
> > +
> > +#define AXP288_EXTCON_CABLE_SDP "Slow-charger"
> > +#define AXP288_EXTCON_CABLE_CDP "Charge-downstream"
> > +#define AXP288_EXTCON_CABLE_DCP "Fast-charger"
>
> I'll rename the cable name by using capital letter instead of small letter.
> So, I'd like you to rename cable as following:
> - "Slow-charger" -> "SLOW-CHARGER"
> - "Charge-downstream" ->"CHARGE-DOWNSTREAM"
> - "Fast-charger" -> "FAST-CHARGER"
Ok I will change.
> > +
> > +enum axp288_mux_select {
> > + EXTCON_GPIO_MUX_SEL_PMIC = 0,
> > + EXTCON_GPIO_MUX_SEL_SOC,
> > +};
> > +
> > +enum axp288_extcon_irq {
> > + VBUS_FALLING_IRQ = 0,
> > + VBUS_RISING_IRQ,
> > + MV_CHNG_IRQ,
> > + BC_USB_CHNG_IRQ,
> > + EXTCON_IRQ_END,
> > +};
> > +
> > +static const char *axp288_extcon_cables[] = {
> > + AXP288_EXTCON_CABLE_SDP,
> > + AXP288_EXTCON_CABLE_CDP,
> > + AXP288_EXTCON_CABLE_DCP,
> > + NULL,
> > +};
> > +
> > +struct axp288_extcon_info {
> > + struct device *dev;
> > + struct regmap *regmap;
> > + struct regmap_irq_chip_data *regmap_irqc;
> > + struct axp288_extcon_pdata *pdata;
> > + int irq[EXTCON_IRQ_END];
> > + struct extcon_dev *edev;
> > + struct usb_phy *otg;
> > + struct notifier_block extcon_nb;
> > + struct extcon_specific_cable_nb cable;
> > + /* detect OTG or ID events */
>
> This driver have the feature of both extcon provider driver and extcon consumer
> driver.
Yes, it's acts as both...I will remove the consumer support from this patch as you suggested in later comments.
>
> > + bool usb_id_short;
> > +};
> > +
> > +/* Power up/down reason string array */ static char
> > +*axp288_pwr_up_down_info[] = {
> > + "Last wake caused by user pressing the power button",
> > + "Last wake caused by a charger insertion",
> > + "Last wake caused by a battery insertion",
> > + "Last wake caused by SOC initiated global reset",
> > + "Last wake caused by cold reset",
> > + "Last shutdown caused by PMIC UVLO threshold",
> > + "Last shutdown caused by SOC initiated cold off",
> > + "Last shutdown caused by user pressing the power button",
> > + NULL,
> > +};
> > +
> > +/*
> > + * Decode and log the given "reset source indicator" (rsi)
> > + * register and then clear it.
> > + */
> > +static void axp288_extcon_log_rsi(struct axp288_extcon_info *info,
> > + char **pwrsrc_rsi_info, int reg)
>
> This function read only AXP288_PS_BOOT_REASON_REG register.
> So, axp288_extcon_log_rsi() don't need the second parameter (int reg).
Ok. I will fix this.
> > +{
> > + char **rsi;
> > + unsigned int val, i, clear_mask = 0;
> > + int ret;
> > +
> > + ret = regmap_read(info->regmap, reg, &val);
> > + for (i = 0, rsi = pwrsrc_rsi_info; *rsi; rsi++, i++) {
> > + if (val & BIT(i)) {
> > + dev_dbg(info->dev, "%s\n", *rsi);
> > + clear_mask |= BIT(i);
> > + }
> > + }
> > +
> > + /* Clear the register value for next reboot (write 1 to clear bit) */
> > + regmap_write(info->regmap, reg, clear_mask); }
> > +
> > +static int handle_chrg_det_event(struct axp288_extcon_info *info) {
> > + static bool notify_otg, notify_charger;
> > + static char *cable;
> > + int ret, stat, cfg, pwr_stat;
> > + u8 chrg_type;
> > + bool vbus_attach = false;
> > +
> > + ret = regmap_read(info->regmap, AXP288_PS_STAT_REG, &pwr_stat);
> > + if (ret < 0) {
> > + dev_err(info->dev, "vbus status read error\n");
> > + return ret;
> > + }
> > +
> > + vbus_attach = (pwr_stat & PS_STAT_VBUS_PRESENT) && !info-
> >usb_id_short;
> > + if (vbus_attach) {
> > + dev_dbg(info->dev, "vbus present\n");
> > + } else {
> > + dev_dbg(info->dev, "vbus not present\n");
> > + goto notify_otg;
> > + }
> > +
> > + /* Check charger detection completion status */
> > + ret = regmap_read(info->regmap, AXP288_BC_GLOBAL_REG, &cfg);
> > + if (ret < 0)
> > + goto dev_det_ret;
> > + if (cfg & BC_GLOBAL_DET_STAT) {
> > + dev_dbg(info->dev, "charger detection not complete!!\n");
> > + goto dev_det_ret;
> > + }
> > +
> > + ret = regmap_read(info->regmap, AXP288_BC_DET_STAT_REG, &stat);
> > + if (ret < 0)
> > + goto dev_det_ret;
> > + dev_dbg(info->dev, "stat:%x, cfg:%x\n", stat, cfg);
> > +
> > + chrg_type = (stat & DET_STAT_MASK) >> DET_STAT_SHIFT;
> > +
> > + if (chrg_type == DET_STAT_SDP) {
> > + dev_dbg(info->dev, "sdp cable connecetd\n");
> > + notify_otg = true;
> > + notify_charger = true;
> > + cable = AXP288_EXTCON_CABLE_SDP;
> > + } else if (chrg_type == DET_STAT_CDP) {
> > + dev_dbg(info->dev, "cdp cable connecetd\n");
> > + notify_otg = true;
> > + notify_charger = true;
> > + cable = AXP288_EXTCON_CABLE_CDP;
> > + } else if (chrg_type == DET_STAT_DCP) {
> > + dev_dbg(info->dev, "dcp cable connecetd\n");
> > + notify_charger = true;
> > + cable = AXP288_EXTCON_CABLE_DCP;
> > + } else {
> > + dev_warn(info->dev,
> > + "disconnect or unknown or ID event\n");
> > + }
> > +
> > +notify_otg:
> > + if (notify_otg) {
> > + /*
> > + * If VBUS is absent Connect D+/D- lines to PMIC for BC
> > + * detection. Else connect them to SOC for USB communication.
> > + */
> > + if (info->pdata->gpio_mux_cntl != NULL)
> > + gpiod_set_value(info->pdata->gpio_mux_cntl,
> > + vbus_attach ? EXTCON_GPIO_MUX_SEL_SOC
> > + :
> EXTCON_GPIO_MUX_SEL_PMIC);
> > +
> > + atomic_notifier_call_chain(&info->otg->notifier,
> > + vbus_attach ? USB_EVENT_VBUS : USB_EVENT_NONE,
> NULL);
> > + }
> > +
> > + if (notify_charger)
> > + extcon_set_cable_state(info->edev, cable, vbus_attach);
> > +
> > + /* Clear the flags on disconnect event */
> > + if (!vbus_attach) {
> > + notify_otg = false;
> > + notify_charger = false;
> > + }
> > +
> > + return 0;
> > +
> > +dev_det_ret:
> > + if (ret < 0)
> > + dev_warn(info->dev, "BC Mod detection error\n");
> > +
> > + return ret;
> > +}
> > +
> > +static irqreturn_t axp288_extcon_isr(int irq, void *data) {
> > + struct axp288_extcon_info *info = data;
> > + unsigned int i;
> > + int ret;
> > +
> > + for (i = 0; i < EXTCON_IRQ_END; i++) {
> > + if (info->irq[i] == irq)
> > + break;
> > + }
> > +
> > + if (i == EXTCON_IRQ_END) {
> > + dev_warn(info->dev, "spurious interrupt!!\n");
> > + return IRQ_NONE;
> > + }
> > +
> > + ret = handle_chrg_det_event(info);
> > + if (ret < 0)
> > + dev_warn(info->dev, "error in PWRSRC INT handling\n");
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int axp288_handle_extcon_event(struct notifier_block *nb,
> > + unsigned long event, void *param) {
> > + struct axp288_extcon_info *info =
> > + container_of(nb, struct axp288_extcon_info, extcon_nb);
> > + struct extcon_dev *edev = param;
> > + int usb_host = extcon_get_cable_state(edev, "USB-Host");
> > +
> > + dev_dbg(info->dev, "external connector USB-Host is %s\n",
> > + usb_host ? "attached" : "detached");
> > +
> > + /*
> > + * Set usb_id_short flag to avoid running charger detection logic
> > + * in case usb host.
> > + */
> > + info->usb_id_short = usb_host;
> > +
> > + /*
> > + * Connect the USB mux to SOC in case of usb host else connect
> > + * it to PMIC.
> > + */
> > + if (info->pdata->gpio_mux_cntl != NULL) {
> > + if (info->usb_id_short)
> > + gpiod_set_value(info->pdata->gpio_mux_cntl,
> > + EXTCON_GPIO_MUX_SEL_SOC);
> > + else
> > + gpiod_set_value(info->pdata->gpio_mux_cntl,
> > + EXTCON_GPIO_MUX_SEL_PMIC);
> > + }
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +static int axp288_init_gpio_mux_cntl(struct axp288_extcon_info *info)
> > +{
> > + int ret;
> > +
> > + ret = gpio_request(desc_to_gpio(info->pdata->gpio_mux_cntl),
> "USB_MUX");
> > + if (ret < 0) {
> > + dev_err(info->dev, "Failed to request the gpio=%d\n",
> > + desc_to_gpio(info->pdata->gpio_mux_cntl));
> > + return ret;
> > + }
> > + gpiod_direction_output(info->pdata->gpio_mux_cntl,
> > + EXTCON_GPIO_MUX_SEL_PMIC);
> > +
> > + info->extcon_nb.notifier_call = axp288_handle_extcon_event;
> > + ret = extcon_register_interest(&info->cable, NULL,
> > + "USB-Host", &info->extcon_nb);
> > + if (ret) {
> > + dev_err(info->dev, "failed to register extcon notifier\n");
> > + return ret;
> > + }
> > +
> > + if (info->cable.edev)
> > + info->usb_id_short =
> > + extcon_get_cable_state(info->cable.edev, "USB-Host");
> > +
> > + if (info->usb_id_short)
> > + gpiod_set_value(info->pdata->gpio_mux_cntl,
> > + EXTCON_GPIO_MUX_SEL_SOC);
> > +
> > + return 0;
> > +}
>
> Currently, the only extcon provider driver are included in drivers/extcon.
> I'm reruclant to support extcon consumer feature in extcon directory.
> Also, this driver support both extcon provider feature and extcon consumer
> feature in one driver. It is one reluctant reason.
>
> So, I suggest that you includes only extcon provider feature in this patchset.
> And then we will discuss the support of extcon consumer feature on later patch.
>
> I will support the 'usb notifier chain' to detect both vbus and id pin state.
> you can refet to this discussion on patch[1].
> #define EXTCON_USB_ID_LOW
> #define EXTCON_USB_ID_HIGH
> #define EXTCON_USB_VBUS_LOW
> #define EXTCON_USB_VBUS_HIGH
>
> [1] https://lkml.org/lkml/2015/4/16/116
Ok..fine.
> > +
> > +static int axp288_extcon_probe(struct platform_device *pdev) {
> > + struct axp288_extcon_info *info;
> > + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> > + int ret, i, pirq;
> > +
> > + info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> > + if (!info)
> > + return -ENOMEM;
> > +
> > + info->dev = &pdev->dev;
> > + info->regmap = axp20x->regmap;
> > + info->regmap_irqc = axp20x->regmap_irqc;
> > + info->pdata = pdev->dev.platform_data;
> > +
> > + if (!info->pdata) {
> > + /* TODO: Try ACPI provided pdata via device properties */
> > + if (!device_property_present(&pdev->dev,
> > + "axp288_extcon_data\n"))
> > + dev_err(&pdev->dev, "no platform data\n");
> > + return -ENODEV;
> > + }
> > + platform_set_drvdata(pdev, info);
> > +
> > + axp288_extcon_log_rsi(info, axp288_pwr_up_down_info,
> > + AXP288_PS_BOOT_REASON_REG);
> > +
> > + /* Initialize extcon device */
> > + info->edev = devm_extcon_dev_allocate(&pdev->dev,
> > + axp288_extcon_cables);
> > + if (IS_ERR(info->edev)) {
> > + dev_err(&pdev->dev, "failed to allocate memory for extcon\n");
> > + return -ENOMEM;
>
> Use PTR_ERR(info->edev) instead of -ENOMEM
Ok..
>
> > + }
> > +
> > + /* Register extcon device */
> > + info->edev->name = dev_name(&pdev->dev);
>
> Don't need to set the name of extcon device. extcon_dev_register() set the
> name of extcon device. So, I'll move the extcon_dev structure from extcon.h to
> extcon.c becaseu extcon_dev structure should be handled in extcon core.
Ok..
>
> > + ret = devm_extcon_dev_register(&pdev->dev, info->edev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to register extcon device\n");
> > + return ret;
> > + }
> > +
> > + /* Get otg transceiver phy */
> > + info->otg = usb_get_phy(USB_PHY_TYPE_USB2);
> > + if (IS_ERR(info->otg)) {
> > + dev_warn(&pdev->dev, "Failed to get otg transceiver!!\n");
>
> 'Failed' -> 'failed'. don't need the '!!' on end of debug log.
Ok..
>
> > + ret = PTR_ERR(info->otg);
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < EXTCON_IRQ_END; i++) {
> > + pirq = platform_get_irq(pdev, i);
> > + info->irq[i] = regmap_irq_get_virq(info->regmap_irqc, pirq);
> > + if (info->irq[i] < 0) {
> > + dev_warn(&pdev->dev,
> > + "regmap_irq get virq failed for IRQ %d: %d\n",
> > + pirq, info->irq[i]);
> > + info->irq[i] = -1;
> > + goto intr_reg_failed;
> > + }
>
> Need to add blank line
Not sure how it came here...i don't see the line in vim
>
> > + ret = devm_request_threaded_irq(&pdev->dev, info->irq[i],
> > + NULL, axp288_extcon_isr,
> > + IRQF_ONESHOT | IRQF_NO_SUSPEND,
> > + pdev->name, info);
> > + if (ret) {
> > + dev_err(&pdev->dev, "request_irq fail :%d err:%d\n",
> > + info->irq[i], ret);
> > + goto intr_reg_failed;
> > + }
> > + }
> > +
> > + /* Set up gpio control for USB Mux */
> > + if (info->pdata->gpio_mux_cntl != NULL) {
>
> Modify if statement as following simply:
> if (info->pdata->gpio_mux_cntl)
Ok..
>
> > + ret = axp288_init_gpio_mux_cntl(info);
> > + if (ret < 0)
> > + goto intr_reg_failed;
> > + }
> > +
> > + /* Unmask VBUS interrupt */
> > + regmap_write(info->regmap, AXP288_PWRSRC_IRQ_CFG_REG,
> > + PWRSRC_IRQ_CFG_MASK);
> > + regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
> > + BC_GLOBAL_RUN, 0);
> > + /* Unmask the BC1.2 complte interrupts */
> > + regmap_write(info->regmap, AXP288_BC12_IRQ_CFG_REG,
> BC12_IRQ_CFG_MASK);
> > + /* Enable the charger detection logic */
> > + regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
> > + BC_GLOBAL_RUN, BC_GLOBAL_RUN);
>
> I think that you better to move the initialization code before extcon registration.
> The initialization should be executed before extcon and interrupt registration.
My intention is to enable the interrupts once after setting all the interrupt and extcon handlers.
>
> > +
> > + return 0;
> > +
> > +intr_reg_failed:
> > + usb_put_phy(info->otg);
> > + return ret;
> > +}
> > +
> > +static int axp288_extcon_remove(struct platform_device *pdev) {
> > + struct axp288_extcon_info *info = platform_get_drvdata(pdev);
> > +
> > + usb_put_phy(info->otg);
> > + return 0;
> > +}
> > +
> > +static struct platform_driver axp288_extcon_driver = {
> > + .probe = axp288_extcon_probe,
> > + .remove = axp288_extcon_remove,
> > + .driver = {
> > + .name = "axp288_extcon",
> > + },
>
> You should add the suspend/resume funciton to control the interrupt during
> suspend state. I discussed the same issue on patch[2].
> And you can refer to extcon-usb-gpio.c[3] for controlling interrupt in
> suspend/resume function.
>
> [2] https://lkml.org/lkml/2015/1/27/1069
> [3]
> http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extc
> on-next&id=e52817faae359ce95c93c2b6eb88b16d4b430181
The actual HW interrupt is controlled/handled by mfd driver. This driver only gets the virtual interrupts.
I don't see the need to enable/disable the interrupts in this case...
Thanks,
Ram
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists