* [v3,1/2] dt-bindings: Documentation for qcom,eud @ 2018-11-15 19:32 ` Prakruthi Deepak Heragu 2018-11-17 14:47 ` Rob Herring 0 siblings, 1 reply; 8+ messages in thread From: Prakruthi Deepak Heragu @ 2018-11-15 19:32 UTC (permalink / raw) To: linux-arm-msm, linux-usb, devicetree Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Prakruthi Deepak Heragu, Satya Durga Srinivasu Prabhala Documentation for Embedded USB Debugger (EUD) device tree bindings. Signed-off-by: Satya Durga Srinivasu Prabhala <satyap@codeaurora.org> Signed-off-by: Prakruthi Deepak Heragu <pheragu@codeaurora.org> --- .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file mode 100644 index 0000000..6d339e7 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt @@ -0,0 +1,43 @@ +* Qualcomm Technologies Inc Embedded USB Debugger (EUD) + +The EUD (Embedded USB Debugger) is a mini-USB hub implemented +on chip to support the USB-based debug and trace capabilities. + +Required properties: + + - compatible: Should be "qcom,msm-eud" + - interrupts: Interrupt number + - reg: Should be address and size of EUD register space + +EUD notifies clients for VBUS attach/detach and charger enable/disable +events. The link between client and EUD is established via a directed +graph. EUD has one endpoint of the graph link mentioning EUD as an +output link and clients of EUD should have the other endpoint of the +graph link as an input link. + +An example for EUD device node: + + eud: qcom,msm-eud@88e0000 { + compatible = "qcom,msm-eud"; + interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>; + reg = <0x88e0000 0x4000>; + usb_con: connector { + compatible = "usb-c-connector"; + label = "USB-C"; + port { + eud_usb_output: endpoint { + remote-endpoint = <&eud_usb3_input>; + }; + }; + }; + }; + +An example for EUD client: + + usb3 { + port { + eud_usb3_input: endpoint { + remote-endpoint = <&eud_usb_output>; + }; + }; + }; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [v3,1/2] dt-bindings: Documentation for qcom,eud @ 2018-11-17 14:47 ` Rob Herring 2020-01-09 14:58 ` [PATCH v3 1/2] " Dwivedi, Avaneesh Kumar (avani) 0 siblings, 1 reply; 8+ messages in thread From: Rob Herring @ 2018-11-17 14:47 UTC (permalink / raw) To: Prakruthi Deepak Heragu Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Satya Durga Srinivasu Prabhala On Thu, Nov 15, 2018 at 11:32:53AM -0800, Prakruthi Deepak Heragu wrote: > Documentation for Embedded USB Debugger (EUD) device tree bindings. I asked questions on v2 which were never answered. > > Signed-off-by: Satya Durga Srinivasu Prabhala <satyap@codeaurora.org> > Signed-off-by: Prakruthi Deepak Heragu <pheragu@codeaurora.org> > --- > .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 43 ++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt > new file mode 100644 > index 0000000..6d339e7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt > @@ -0,0 +1,43 @@ > +* Qualcomm Technologies Inc Embedded USB Debugger (EUD) > + > +The EUD (Embedded USB Debugger) is a mini-USB hub implemented > +on chip to support the USB-based debug and trace capabilities. > + > +Required properties: > + > + - compatible: Should be "qcom,msm-eud" > + - interrupts: Interrupt number > + - reg: Should be address and size of EUD register space > + > +EUD notifies clients for VBUS attach/detach and charger enable/disable > +events. The link between client and EUD is established via a directed > +graph. EUD has one endpoint of the graph link mentioning EUD as an > +output link and clients of EUD should have the other endpoint of the > +graph link as an input link. > + > +An example for EUD device node: > + > + eud: qcom,msm-eud@88e0000 { > + compatible = "qcom,msm-eud"; > + interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>; > + reg = <0x88e0000 0x4000>; > + usb_con: connector { > + compatible = "usb-c-connector"; > + label = "USB-C"; > + port { > + eud_usb_output: endpoint { > + remote-endpoint = <&eud_usb3_input>; > + }; > + }; > + }; > + }; > + > +An example for EUD client: > + > + usb3 { > + port { > + eud_usb3_input: endpoint { > + remote-endpoint = <&eud_usb_output>; > + }; > + }; > + }; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: Documentation for qcom,eud 2018-11-17 14:47 ` Rob Herring @ 2020-01-09 14:58 ` Dwivedi, Avaneesh Kumar (avani) 0 siblings, 0 replies; 8+ messages in thread From: Dwivedi, Avaneesh Kumar (avani) @ 2020-01-09 14:58 UTC (permalink / raw) To: Rob Herring, Prakruthi Deepak Heragu Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Satya Durga Srinivasu Prabhala On 11/17/2018 8:17 PM, Rob Herring wrote: > On Thu, Nov 15, 2018 at 11:32:53AM -0800, Prakruthi Deepak Heragu wrote: >> Documentation for Embedded USB Debugger (EUD) device tree bindings. > I asked questions on v2 which were never answered. HI Rob, Can you please review my replies against your comments on patchset v2? > >> Signed-off-by: Satya Durga Srinivasu Prabhala <satyap@codeaurora.org> >> Signed-off-by: Prakruthi Deepak Heragu <pheragu@codeaurora.org> >> --- >> .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 43 ++++++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt >> new file mode 100644 >> index 0000000..6d339e7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt >> @@ -0,0 +1,43 @@ >> +* Qualcomm Technologies Inc Embedded USB Debugger (EUD) >> + >> +The EUD (Embedded USB Debugger) is a mini-USB hub implemented >> +on chip to support the USB-based debug and trace capabilities. >> + >> +Required properties: >> + >> + - compatible: Should be "qcom,msm-eud" >> + - interrupts: Interrupt number >> + - reg: Should be address and size of EUD register space >> + >> +EUD notifies clients for VBUS attach/detach and charger enable/disable >> +events. The link between client and EUD is established via a directed >> +graph. EUD has one endpoint of the graph link mentioning EUD as an >> +output link and clients of EUD should have the other endpoint of the >> +graph link as an input link. >> + >> +An example for EUD device node: >> + >> + eud: qcom,msm-eud@88e0000 { >> + compatible = "qcom,msm-eud"; >> + interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>; >> + reg = <0x88e0000 0x4000>; >> + usb_con: connector { >> + compatible = "usb-c-connector"; >> + label = "USB-C"; >> + port { >> + eud_usb_output: endpoint { >> + remote-endpoint = <&eud_usb3_input>; >> + }; >> + }; >> + }; >> + }; >> + >> +An example for EUD client: >> + >> + usb3 { >> + port { >> + eud_usb3_input: endpoint { >> + remote-endpoint = <&eud_usb_output>; >> + }; >> + }; >> + }; >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, >> a Linux Foundation Collaborative Project >> -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [v3,2/2] Embedded USB Debugger (EUD) driver @ 2018-11-15 19:32 ` Prakruthi Deepak Heragu 2018-11-15 23:06 ` Greg Kroah-Hartman 0 siblings, 1 reply; 8+ messages in thread From: Prakruthi Deepak Heragu @ 2018-11-15 19:32 UTC (permalink / raw) To: linux-arm-msm, linux-usb, devicetree Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Prakruthi Deepak Heragu, Satya Durga Srinivasu Prabhala Add support for control peripheral of EUD (Embedded USB Debugger) to listen to events such as USB attach/detach, charger enable/disable, pet EUD to indicate software is functional. Reusing the platform device kobj, sysfs entry 'enable' is created to enable or disable EUD. Signed-off-by: Satya Durga Srinivasu Prabhala <satyap@codeaurora.org> Signed-off-by: Prakruthi Deepak Heragu <pheragu@codeaurora.org> --- drivers/soc/qcom/Kconfig | 12 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 357 insertions(+) create mode 100644 drivers/soc/qcom/eud.c diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 5856e79..12669ec 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -136,4 +136,16 @@ config QCOM_APR application processor and QDSP6. APR is used by audio driver to configure QDSP6 ASM, ADM and AFE modules. + +config QCOM_EUD + tristate "QTI Embedded USB Debugger (EUD)" + depends on ARCH_QCOM + help + The Embedded USB Debugger (EUD) driver is a driver for the + control peripheral which waits on events like USB attach/detach + and charger enable/disable. The control peripheral further helps + support the USB-based debug and trace capabilities. + This module enables support for Qualcomm Technologies, Inc. + Embedded USB Debugger (EUD). + If unsure, say N. endmenu diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 19dcf95..dd4701b 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -15,3 +15,4 @@ obj-$(CONFIG_QCOM_SMP2P) += smp2p.o obj-$(CONFIG_QCOM_SMSM) += smsm.o obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o obj-$(CONFIG_QCOM_APR) += apr.o +obj-$(CONFIG_QCOM_EUD) += eud.o diff --git a/drivers/soc/qcom/eud.c b/drivers/soc/qcom/eud.c new file mode 100644 index 0000000..bd885d6 --- /dev/null +++ b/drivers/soc/qcom/eud.c @@ -0,0 +1,344 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + */ + +#include <linux/kernel.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/interrupt.h> +#include <linux/err.h> +#include <linux/platform_device.h> +#include <linux/extcon.h> +#include <linux/delay.h> +#include <linux/sysfs.h> +#include <linux/io.h> +#include <linux/bitops.h> +#include <linux/workqueue.h> +#include <linux/power_supply.h> + +#define EUD_ENABLE_CMD 1 +#define EUD_DISABLE_CMD 0 + +#define EUD_REG_INT1_EN_MASK 0x0024 +#define EUD_REG_INT_STATUS_1 0x0044 +#define EUD_REG_CTL_OUT_1 0x0074 +#define EUD_REG_VBUS_INT_CLR 0x0080 +#define EUD_REG_CHGR_INT_CLR 0x0084 +#define EUD_REG_CSR_EUD_EN 0x1014 +#define EUD_REG_SW_ATTACH_DET 0x1018 + +#define EUD_INT_VBUS BIT(2) +#define EUD_INT_CHGR BIT(3) +#define EUD_INT_SAFE_MODE BIT(4) +#define EUD_INT_ALL (EUD_INT_VBUS|EUD_INT_CHGR|\ + EUD_INT_SAFE_MODE) + +struct eud_chip { + struct device *dev; + int eud_irq; + unsigned int extcon_id; + unsigned int int_status; + bool usb_attach; + bool chgr_enable; + void __iomem *eud_reg_base; + struct extcon_dev *extcon; + int enable; + struct work_struct eud_work; +}; + +static const unsigned int eud_extcon_cable[] = { + EXTCON_USB, + EXTCON_CHG_USB_SDP, + EXTCON_NONE, +}; + +static int enable_eud(struct eud_chip *priv) +{ + struct power_supply *usb_psy = NULL; + union power_supply_propval pval = {0}; + union power_supply_propval tval = {0}; + int ret; + + usb_psy = power_supply_get_by_name("usb"); + if (!usb_psy) + return -1; + + ret = power_supply_get_property(usb_psy, + POWER_SUPPLY_PROP_PRESENT, &pval); + if (ret) + return ret; + + ret = power_supply_get_property(usb_psy, + POWER_SUPPLY_PROP_REAL_TYPE, &tval); + if (ret) + return ret; + + if (!pval.intval || (tval.intval != POWER_SUPPLY_TYPE_USB && + tval.intval != POWER_SUPPLY_TYPE_USB_CDP)) + return -1; + + /* write into CSR to enable EUD */ + writel_relaxed(BIT(0), priv->eud_reg_base + EUD_REG_CSR_EUD_EN); + /* Enable vbus, chgr & safe mode warning interrupts */ + writel_relaxed(EUD_INT_VBUS | EUD_INT_CHGR | EUD_INT_SAFE_MODE, + priv->eud_reg_base + EUD_REG_INT1_EN_MASK); + + /* Ensure Register Writes Complete */ + wmb(); + + /* + * Set the default cable state to usb connect and charger + * enable + */ + ret = extcon_set_state_sync(priv->extcon, EXTCON_USB, true); + if (ret) + return ret; + ret = extcon_set_state_sync(priv->extcon, + EXTCON_CHG_USB_SDP, true); + if (ret) + return ret; + + return 0; +} + +static void disable_eud(struct eud_chip *priv) +{ + /* write into CSR to disable EUD */ + writel_relaxed(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN); +} + +static ssize_t eud_enable_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct eud_chip *chip = dev_get_drvdata(dev); + + return snprintf(buf, sizeof(int), "%d", chip->enable); +} + +static ssize_t eud_enable_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct eud_chip *chip = dev_get_drvdata(dev); + int enable = 0; + int ret = 0; + + if (sscanf(buf, "%du", &enable) != 1) + return -EINVAL; + + if (enable == EUD_ENABLE_CMD) + ret = enable_eud(chip); + else if (enable == EUD_DISABLE_CMD) + disable_eud(chip); + if (!ret) + chip->enable = enable; + return count; +} + +struct device_attribute eud_attribute = { + .attr.name = "enable", + .attr.mode = 0644, + .show = eud_enable_show, + .store = eud_enable_store, +}; + +static void eud_event_notifier(struct work_struct *eud_work) +{ + struct eud_chip *chip = container_of(eud_work, struct eud_chip, + eud_work); + int ret; + + if (chip->int_status == EUD_INT_VBUS) { + ret = extcon_set_state_sync(chip->extcon, chip->extcon_id, + chip->usb_attach); + if (ret) + return; + } else if (chip->int_status == EUD_INT_CHGR) { + ret = extcon_set_state_sync(chip->extcon, chip->extcon_id, + chip->chgr_enable); + if (ret) + return; + } +} + +static void usb_attach_detach(struct eud_chip *chip) +{ + u32 reg; + + chip->extcon_id = EXTCON_USB; + /* read ctl_out_1[4] to find USB attach or detach event */ + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1); + if (reg & BIT(4)) + chip->usb_attach = true; + else + chip->usb_attach = false; + + schedule_work(&chip->eud_work); + + /* set and clear vbus_int_clr[0] to clear interrupt */ + writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR); + /* Ensure Register Writes Complete */ + wmb(); + writel_relaxed(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR); +} + +static void chgr_enable_disable(struct eud_chip *chip) +{ + u32 reg; + + chip->extcon_id = EXTCON_CHG_USB_SDP; + /* read ctl_out_1[6] to find charger enable or disable event */ + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1); + if (reg & BIT(6)) + chip->chgr_enable = true; + else + chip->chgr_enable = false; + + schedule_work(&chip->eud_work); + + /* set and clear chgr_int_clr[0] to clear interrupt */ + writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_CHGR_INT_CLR); + /* Ensure Register Writes Complete */ + wmb(); + writel_relaxed(0, chip->eud_reg_base + EUD_REG_CHGR_INT_CLR); +} + +static void pet_eud(struct eud_chip *chip) +{ + u32 reg; + + /* read sw_attach_det[0] to find attach/detach event */ + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET); + if (reg & BIT(0)) { + /* Detach & Attach pet for EUD */ + writel_relaxed(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET); + /* Ensure Register Writes Complete */ + wmb(); + /* Delay to make sure detach pet is done before attach pet */ + udelay(100); + writel_relaxed(BIT(0), chip->eud_reg_base + + EUD_REG_SW_ATTACH_DET); + /* Ensure Register Writes Complete */ + wmb(); + } else { + /* Attach pet for EUD */ + writel_relaxed(BIT(0), chip->eud_reg_base + + EUD_REG_SW_ATTACH_DET); + /* Ensure Register Writes Complete */ + wmb(); + } +} + +static irqreturn_t handle_eud_irq(int irq, void *data) +{ + struct eud_chip *chip = data; + u32 reg; + + /* read status register and find out which interrupt triggered */ + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_INT_STATUS_1); + switch (reg & EUD_INT_ALL) { + case EUD_INT_VBUS: + chip->int_status = EUD_INT_VBUS; + usb_attach_detach(chip); + break; + case EUD_INT_CHGR: + chip->int_status = EUD_INT_CHGR; + chgr_enable_disable(chip); + break; + case EUD_INT_SAFE_MODE: + pet_eud(chip); + break; + default: + return IRQ_NONE; + } + return IRQ_HANDLED; +} + +static int msm_eud_probe(struct platform_device *pdev) +{ + struct eud_chip *chip; + struct resource *res; + int ret; + + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL); + if (!chip) + return -ENOMEM; + + chip->dev = &pdev->dev; + platform_set_drvdata(pdev, chip); + + ret = device_create_file(&pdev->dev, &eud_attribute); + if (ret) + return ret; + + chip->extcon = devm_extcon_dev_allocate(&pdev->dev, eud_extcon_cable); + if (IS_ERR(chip->extcon)) + return PTR_ERR(chip->extcon); + + ret = devm_extcon_dev_register(&pdev->dev, chip->extcon); + if (ret) + return ret; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENOMEM; + + chip->eud_reg_base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(chip->eud_reg_base)) + return PTR_ERR(chip->eud_reg_base); + + chip->eud_irq = platform_get_irq(pdev, 0); + + ret = devm_request_irq(&pdev->dev, chip->eud_irq, handle_eud_irq, + IRQF_TRIGGER_HIGH, NULL, chip); + if (ret) + return ret; + + device_init_wakeup(&pdev->dev, true); + enable_irq_wake(chip->eud_irq); + + INIT_WORK(&chip->eud_work, eud_event_notifier); + + if (ret) + return ret; + + /* Enable EUD */ + if (chip->enable) + enable_eud(chip); + + return 0; +} + +static int msm_eud_remove(struct platform_device *pdev) +{ + struct eud_chip *chip = platform_get_drvdata(pdev); + + if (chip->enable) + disable_eud(chip); + device_remove_file(&pdev->dev, &eud_attribute); + device_init_wakeup(&pdev->dev, false); + disable_irq_wake(chip->eud_irq); + + return 0; +} + +static const struct of_device_id msm_eud_dt_match[] = { + {.compatible = "qcom,msm-eud"}, + {}, +}; +MODULE_DEVICE_TABLE(of, msm_eud_dt_match); + +static struct platform_driver msm_eud_driver = { + .probe = msm_eud_probe, + .remove = msm_eud_remove, + .driver = { + .name = "msm-eud", + .of_match_table = msm_eud_dt_match, + }, +}; +module_platform_driver(msm_eud_driver); + +MODULE_DESCRIPTION("QTI EUD driver"); +MODULE_LICENSE("GPL v2"); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [v3,2/2] Embedded USB Debugger (EUD) driver @ 2018-11-15 23:06 ` Greg Kroah-Hartman 2020-01-23 21:41 ` [PATCH v3 2/2] " Dwivedi, Avaneesh Kumar (avani) 0 siblings, 1 reply; 8+ messages in thread From: Greg Kroah-Hartman @ 2018-11-15 23:06 UTC (permalink / raw) To: Prakruthi Deepak Heragu Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Satya Durga Srinivasu Prabhala On Thu, Nov 15, 2018 at 11:32:54AM -0800, Prakruthi Deepak Heragu wrote: > Add support for control peripheral of EUD (Embedded USB Debugger) to > listen to events such as USB attach/detach, charger enable/disable, pet > EUD to indicate software is functional. Reusing the platform device kobj, > sysfs entry 'enable' is created to enable or disable EUD. If you add/remove/change a sysfs file, you need to also have a Documentation/ABI/ file update as well. Please do that here. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver 2018-11-15 23:06 ` Greg Kroah-Hartman @ 2020-01-23 21:41 ` Dwivedi, Avaneesh Kumar (avani) 2020-01-24 5:57 ` Greg KH 0 siblings, 1 reply; 8+ messages in thread From: Dwivedi, Avaneesh Kumar (avani) @ 2020-01-23 21:41 UTC (permalink / raw) To: Greg KH, Prakruthi Deepak Heragu Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Satya Durga Srinivasu Prabhala On 11/16/2018 4:36 AM, Greg KH wrote: > On Thu, Nov 15, 2018 at 11:32:54AM -0800, Prakruthi Deepak Heragu wrote: >> Add support for control peripheral of EUD (Embedded USB Debugger) to >> listen to events such as USB attach/detach, charger enable/disable, pet >> EUD to indicate software is functional. Reusing the platform device kobj, >> sysfs entry 'enable' is created to enable or disable EUD. > If you add/remove/change a sysfs file, you need to also have a > Documentation/ABI/ file update as well. Please do that here. > > thanks, > > greg k-h Thank you very much Greg for your time to review, Shall i go ahead posting next patch set v4 addressing your comments? I was awaiting Rob's review of patch set v3 and also have posted replies to his comments on patch setv-v2. -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver 2020-01-23 21:41 ` [PATCH v3 2/2] " Dwivedi, Avaneesh Kumar (avani) @ 2020-01-24 5:57 ` Greg KH 0 siblings, 0 replies; 8+ messages in thread From: Greg KH @ 2020-01-24 5:57 UTC (permalink / raw) To: Dwivedi, Avaneesh Kumar (avani) Cc: Prakruthi Deepak Heragu, linux-arm-msm, linux-usb, devicetree, linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Satya Durga Srinivasu Prabhala On Fri, Jan 24, 2020 at 03:11:29AM +0530, Dwivedi, Avaneesh Kumar (avani) wrote: > > On 11/16/2018 4:36 AM, Greg KH wrote: > > On Thu, Nov 15, 2018 at 11:32:54AM -0800, Prakruthi Deepak Heragu wrote: > > > Add support for control peripheral of EUD (Embedded USB Debugger) to > > > listen to events such as USB attach/detach, charger enable/disable, pet > > > EUD to indicate software is functional. Reusing the platform device kobj, > > > sysfs entry 'enable' is created to enable or disable EUD. > > If you add/remove/change a sysfs file, you need to also have a > > Documentation/ABI/ file update as well. Please do that here. > > > > thanks, > > > > greg k-h > > Thank you very much Greg for your time to review, Shall i go ahead posting > next patch set v4 addressing your comments? Why wouldn't you? :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [v3,2/2] Embedded USB Debugger (EUD) driver @ 2018-11-15 23:09 Greg KH 0 siblings, 0 replies; 8+ messages in thread From: Greg KH @ 2018-11-15 23:09 UTC (permalink / raw) To: Prakruthi Deepak Heragu Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, ckadabi, tsoni, bryanh, psodagud, rnayak, Satya Durga Srinivasu Prabhala On Thu, Nov 15, 2018 at 11:32:54AM -0800, Prakruthi Deepak Heragu wrote: > +struct device_attribute eud_attribute = { > + .attr.name = "enable", > + .attr.mode = 0644, > + .show = eud_enable_show, > + .store = eud_enable_store, > +}; Please use: static DEVICE_ATTR_RW(enable); instead of open-coding all of that mess. Also it makes it static, this should not be a global symbol. > + > +static void eud_event_notifier(struct work_struct *eud_work) > +{ > + struct eud_chip *chip = container_of(eud_work, struct eud_chip, > + eud_work); > + int ret; > + > + if (chip->int_status == EUD_INT_VBUS) { > + ret = extcon_set_state_sync(chip->extcon, chip->extcon_id, > + chip->usb_attach); > + if (ret) > + return; > + } else if (chip->int_status == EUD_INT_CHGR) { > + ret = extcon_set_state_sync(chip->extcon, chip->extcon_id, > + chip->chgr_enable); > + if (ret) > + return; > + } > +} > + > +static void usb_attach_detach(struct eud_chip *chip) > +{ > + u32 reg; > + > + chip->extcon_id = EXTCON_USB; > + /* read ctl_out_1[4] to find USB attach or detach event */ > + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1); > + if (reg & BIT(4)) > + chip->usb_attach = true; > + else > + chip->usb_attach = false; > + > + schedule_work(&chip->eud_work); > + > + /* set and clear vbus_int_clr[0] to clear interrupt */ > + writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR); > + /* Ensure Register Writes Complete */ > + wmb(); > + writel_relaxed(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR); > +} > + > +static void chgr_enable_disable(struct eud_chip *chip) > +{ > + u32 reg; > + > + chip->extcon_id = EXTCON_CHG_USB_SDP; > + /* read ctl_out_1[6] to find charger enable or disable event */ > + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1); > + if (reg & BIT(6)) > + chip->chgr_enable = true; > + else > + chip->chgr_enable = false; > + > + schedule_work(&chip->eud_work); > + > + /* set and clear chgr_int_clr[0] to clear interrupt */ > + writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_CHGR_INT_CLR); > + /* Ensure Register Writes Complete */ > + wmb(); > + writel_relaxed(0, chip->eud_reg_base + EUD_REG_CHGR_INT_CLR); > +} > + > +static void pet_eud(struct eud_chip *chip) > +{ > + u32 reg; > + > + /* read sw_attach_det[0] to find attach/detach event */ > + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET); > + if (reg & BIT(0)) { > + /* Detach & Attach pet for EUD */ > + writel_relaxed(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET); > + /* Ensure Register Writes Complete */ > + wmb(); > + /* Delay to make sure detach pet is done before attach pet */ > + udelay(100); > + writel_relaxed(BIT(0), chip->eud_reg_base + > + EUD_REG_SW_ATTACH_DET); > + /* Ensure Register Writes Complete */ > + wmb(); > + } else { > + /* Attach pet for EUD */ > + writel_relaxed(BIT(0), chip->eud_reg_base + > + EUD_REG_SW_ATTACH_DET); > + /* Ensure Register Writes Complete */ > + wmb(); > + } > +} > + > +static irqreturn_t handle_eud_irq(int irq, void *data) > +{ > + struct eud_chip *chip = data; > + u32 reg; > + > + /* read status register and find out which interrupt triggered */ > + reg = readl_relaxed(chip->eud_reg_base + EUD_REG_INT_STATUS_1); > + switch (reg & EUD_INT_ALL) { > + case EUD_INT_VBUS: > + chip->int_status = EUD_INT_VBUS; > + usb_attach_detach(chip); > + break; > + case EUD_INT_CHGR: > + chip->int_status = EUD_INT_CHGR; > + chgr_enable_disable(chip); > + break; > + case EUD_INT_SAFE_MODE: > + pet_eud(chip); > + break; > + default: > + return IRQ_NONE; > + } > + return IRQ_HANDLED; > +} > + > +static int msm_eud_probe(struct platform_device *pdev) > +{ > + struct eud_chip *chip; > + struct resource *res; > + int ret; > + > + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL); > + if (!chip) > + return -ENOMEM; > + > + chip->dev = &pdev->dev; > + platform_set_drvdata(pdev, chip); > + > + ret = device_create_file(&pdev->dev, &eud_attribute); > + if (ret) > + return ret; You just raced with userspace and lost :( Isn't there a way to add the attribute to the platform device as a group to have it correctly added by the driver core? Also, you are adding an attribute to a structure that you do not control/own, are you _sure_ you want to do that? It's a bit odd and you don't really control the lifetime of that device. > + > + chip->extcon = devm_extcon_dev_allocate(&pdev->dev, eud_extcon_cable); > + if (IS_ERR(chip->extcon)) > + return PTR_ERR(chip->extcon); As an example of this problem, the sysfs file is now there if this was an error. Yet that sysfs file now means nothing and if userspace touches it bad things will happen. So please at the very least, properly clean up your error paths. And look into doing this correctly. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-24 5:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1542310374-18474-1-git-send-email-pheragu@codeaurora.org> 2018-11-15 19:32 ` [v3,1/2] dt-bindings: Documentation for qcom,eud Prakruthi Deepak Heragu 2018-11-17 14:47 ` Rob Herring 2020-01-09 14:58 ` [PATCH v3 1/2] " Dwivedi, Avaneesh Kumar (avani) 2018-11-15 19:32 ` [v3,2/2] Embedded USB Debugger (EUD) driver Prakruthi Deepak Heragu 2018-11-15 23:06 ` Greg Kroah-Hartman 2020-01-23 21:41 ` [PATCH v3 2/2] " Dwivedi, Avaneesh Kumar (avani) 2020-01-24 5:57 ` Greg KH 2018-11-15 23:09 [v3,2/2] " Greg KH
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).