Add Embedded USB Debugger (EUD) driver
Subsystems (4)
v2 cover Add Embedded USB Debugger (EUD) driver
This is a series of patches that implements a driver for the control peripheral, EUD (Embedded USB Debugger). The EUD is a mini-USB hub implemented on chip to support the USB-based debug and trace capabilities. Apart from debug capabilities, EUD has a control peripheral. Control Peripheral is on when EUD is on and gets signals like USB attach, pet EUD, charge phone etc. EUD driver listens to events like USB attach or detach and charger enable or disable and then notifies the USB driver or PMIC driver respectively about these events via EXTCON. At regular intervals, the EUD driver receives an interrupt to pet the driver indicating that the software is functional.
Changes since v1:
- Remove EUD_NR as it is an unused macro
Changes since v0:
- Remove select SERIAL_CORE from Kconfig as this patch doesn't involve anything related to serial console
- Changed the dt-bindings to remove extcon and replace it with graphs to represent a connection with client
Prakruthi Deepak Heragu (2):
dt-bindings: Documentation for qcom,eud
Embedded USB Debugger (EUD) driver
.../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 41 +++ drivers/soc/qcom/Kconfig | 12 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 338 +++++++++++++++++++++ 4 files changed, 392 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt create mode 100644 drivers/soc/qcom/eud.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 1/2 dt-bindings: Documentation for qcom,eud
Documentation for Embedded USB Debugger (EUD) device tree bindings.
--- .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +41 -0
@@ -0,0 +1,41 @@ +* 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 + +Driver notifies clients for VBUS attach/detach and charger enable/disable +events. The link between client and EUD is established via a directed +graph. EUD driver has one endpoint of the graph link mentioning EUD as an +output link and clients registered for these notifications from the EUD +should have the other endpoint of the graph link as an input link. Each of +these endpoints should contain a 'remote-endpoint' phandle property that +points to the corresponding endpoint in the port of the remote device. + +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>; + port { + eud_output: endpoint { + remote-endpoint = <&usb3_input>; + }; + }; + }; + +An example for EUD client: + + usb3 { + port { + usb3_input: endpoint { + remote-endpoint = <&eud_output>; + }; + }; + };
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Rob Herring
maintainerRe: [PATCH v2 1/2] dt-bindings: Documentation for qcom,eud
Quoted 15 lines · Prakruthi Deepak Heragu
Documentation for Embedded USB Debugger (EUD) device tree bindings.
Signed-off-by Satya Durga Srinivasu Prabhala <[email protected]>Signed-off-by Prakruthi Deepak Heragu <[email protected]>--- .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txtDocumentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +0
-0
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 4
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0@@ -0,0 +1,41 @@ +* 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.
Is it just for debug and normally bypassed?
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 8
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0+ +Required properties: + + - compatible: Should be "qcom,msm-eud"
Needs to be SoC specific (though a fallback is fine).
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 12
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0+ - interrupts: Interrupt number + - reg: Should be address and size of EUD register space + +Driver notifies clients for VBUS attach/detach and charger enable/disable
Bindings are for h/w blocks, not drivers.
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0+events. The link between client and EUD is established via a directed +graph. EUD driver has one endpoint of the graph link mentioning EUD as an +output link and clients registered for these notifications from the EUD +should have the other endpoint of the graph link as an input link.
OF graph is for describing data flows (i.e. h/w connections), not clients wanting some event.
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 18
Quoted 3 lines
Each of
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +2
-0+these endpoints should contain a 'remote-endpoint' phandle property that +points to the corresponding endpoint in the port of the remote device.
You don't need to describe how the graph binding works. Just what the port assignments are.
I worry this is going to collide with using the graph binding for USB connectors.
Quoted 5 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +5
-0+ +An example for EUD device node: + + eud: qcom,msm-eud@88e0000 { + compatible = "qcom,msm-eud";
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 33
Quoted 10 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +10
-0+ interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>; + reg = <0x88e0000 0x4000>; + port { + eud_output: endpoint { + remote-endpoint = <&usb3_input>; + }; + }; + }; + +An example for EUD client:
What are possible clients? Could we want to switch clients at runtime?
Quoted 12 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +8
-0+ + usb3 { + port { + usb3_input: endpoint { + remote-endpoint = <&eud_output>; + }; + }; + };Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 1/2] dt-bindings: Documentation for qcom,eud
Hi Rob,
We would like to revive this thread, We also would like to address your comments on this patch set.
Please consider our reply against your comments so that we can proceed.
Quoted 10 lines · Rob Herring
Quoted 9 lines · Prakruthi Deepak Heragu
Documentation for Embedded USB Debugger (EUD) device tree bindings.
Signed-off-by Satya Durga Srinivasu Prabhala <[email protected]>Signed-off-by Prakruthi Deepak Heragu <[email protected]>--- .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+)create mode 100644
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
Quoted 11 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0@@ -0,0 +1,41 @@ +* 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.Is it just for debug and normally bypassed?
Yes, In normal mode EUD is bypassed.
Quoted 5 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 8
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0+ +Required properties: + + - compatible: Should be "qcom,msm-eud"Needs to be SoC specific (though a fallback is fine).
This IP will be present in all qcom SoC's that is why this is specific to qcom, let us know if this is anyway problematic?
Quoted 5 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 12
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0+ - interrupts: Interrupt number + - reg: Should be address and size of EUD register space + +Driver notifies clients for VBUS attach/detach and charger enable/disableBindings are for h/w blocks, not drivers.
This has been addressed in patch set v3 which is pending for your review.
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
Quoted 5 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +4
-0+events. The link between client and EUD is established via a directed +graph. EUD driver has one endpoint of the graph link mentioning EUD as an +output link and clients registered for these notifications from the EUD +should have the other endpoint of the graph link as an input link.OF graph is for describing data flows (i.e. h/w connections), not clients wanting some event.
Will rephrasing above description as below would work?
"The link between event receiver and EUD is established via a directed graph. Where EUD act as output link and event receiver(ex. usb controller or charger h/w) as input link"
Quoted 5 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 18
Quoted 3 lines
Each of
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +2
-0+these endpoints should contain a 'remote-endpoint' phandle property that +points to the corresponding endpoint in the port of the remote device.You don't need to describe how the graph binding works. Just what the port assignments are.
patch set v3 has removed the part describing how graph binding works.
Quoted 3 lines
I worry this is going to collide with using the graph binding for USB connectors.
Can you please elaborate your query little more? USB controller has input connection from EUD as well as USB connectors.
As mentioned earlier, usb controller receive event from EUD only in debug mode while in normal mode USB connector supplies the event.
does that address concern?
Quoted 17 lines
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +5
-0+ +An example for EUD device node: + + eud: qcom,msm-eud@88e0000 { + compatible = "qcom,msm-eud";Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt line 33
Quoted 10 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +10
-0+ interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>; + reg = <0x88e0000 0x4000>; + port { + eud_output: endpoint { + remote-endpoint = <&usb3_input>; + }; + }; + }; + +An example for EUD client:What are possible clients? Could we want to switch clients at runtime?
As of now clients are usb controller and charger hardware, and they are fixed.
EUD application decide events dynamically.
Quoted 13 lines
Quoted 13 lines
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +8
-0+ + usb3 { + port { + usb3_input: endpoint { + remote-endpoint = <&eud_output>; + }; + }; + };Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v2 2/2 Embedded USB Debugger (EUD) driver
In reply to Prakruthi Deepak HeraguAdd 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.
--- drivers/soc/qcom/Kconfig | 12 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 338 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 drivers/soc/qcom/eud.c
drivers/soc/qcom/Kconfig +12 -0
@@ -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
drivers/soc/qcom/Makefile +1 -0
@@ -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
drivers/soc/qcom/eud.c new file +338 -0
338 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 2/2] Embedded USB Debugger (EUD) driver
Hi,
Quoted 14 lines · Prakruthi Deepak Heragu
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.
Signed-off-by Satya Durga Srinivasu Prabhala <[email protected]>Signed-off-by Prakruthi Deepak Heragu <[email protected]>--- drivers/soc/qcom/Kconfig | 12 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 338 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 drivers/soc/qcom/eud.cdrivers/soc/qcom/Kconfig +0
-0
[snip]
drivers/soc/qcom/Kconfig
Quoted 3 lines
drivers/soc/qcom/Kconfig +3
-0+ +#define EUD_ENABLE_CMD 1 +#define EUD_DISABLE_CMD 0
Why not use module param as boolean? I mean zero to disable and non-zero to enable?
Quoted 50 lines
drivers/soc/qcom/Kconfig +50
-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) + +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; + struct work_struct eud_work; +}; + +static const unsigned int eud_extcon_cable[] = { + EXTCON_USB, + EXTCON_CHG_USB_SDP, + EXTCON_NONE, +}; + +/* + * On the kernel command line specify eud.enable=1 to enable EUD. + * EUD is disabled by default. + */ +static int enable; +static bool eud_ready; +static struct eud_chip *eud_private; + +static void 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; +
drivers/soc/qcom/Kconfig
Quoted 10 lines
drivers/soc/qcom/Kconfig +10
-0+ ret = power_supply_get_property(usb_psy, + POWER_SUPPLY_PROP_PRESENT, &pval); + if (ret) + return; + + ret = power_supply_get_property(usb_psy, + POWER_SUPPLY_PROP_REAL_TYPE, &tval); + if (ret) + return; +
if(!pval.intval || (tval.intval != POWER_SUPPLY_TYPE_USB && tval.intval != POWER_SUPPLY_TYPE_USB_CDP))
return;
/* following if-else can be removed */
Quoted 33 lines
drivers/soc/qcom/Kconfig +33
-0+ if (pval.intval && (tval.intval == POWER_SUPPLY_TYPE_USB || + tval.intval == POWER_SUPPLY_TYPE_USB_CDP)) { + /* 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 = extcon_set_state_sync(priv->extcon, + EXTCON_CHG_USB_SDP, true); + if (ret) + return; + } else { + return; + } + +} + +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); +}
drivers/soc/qcom/Kconfig
Quoted 10 lines
drivers/soc/qcom/Kconfig +10
-0+ +static int param_eud_set(const char *val, const struct kernel_param *kp) +{ + int enable = 0; + + if (sscanf(val, "%du", &enable) != 1) + return -EINVAL; + + if (enable != EUD_ENABLE_CMD && enable != EUD_DISABLE_CMD) + return -EINVAL;
Do we really need to worry about it? 'enable' could just be treated as bool
[snip]
Quoted 10 lines
drivers/soc/qcom/Kconfig +10
-0+ + +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); + if (reg & EUD_INT_VBUS) {
drivers/soc/qcom/Kconfig
Quoted 10 lines
drivers/soc/qcom/Kconfig +10
-0+ chip->int_status = EUD_INT_VBUS; + usb_attach_detach(chip); + } else if (reg & EUD_INT_CHGR) { + chip->int_status = EUD_INT_CHGR; + chgr_enable_disable(chip); + } else if (reg & EUD_INT_SAFE_MODE) { + pet_eud(chip); + } else { + return IRQ_NONE; + }
switch-case instead?
Quoted 4 lines
drivers/soc/qcom/Kconfig +4
-0+ + return IRQ_HANDLED; +} +
drivers/soc/qcom/Kconfig
Quoted 10 lines
drivers/soc/qcom/Kconfig +10
-0+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) { + ret = -ENOMEM; + return ret;
return -ENOMEM;
Quoted 6 lines
drivers/soc/qcom/Kconfig +6
-0+ } + + platform_set_drvdata(pdev, chip); + + chip->extcon = devm_extcon_dev_allocate(&pdev->dev, eud_extcon_cable); + if (IS_ERR(chip->extcon))
drivers/soc/qcom/Kconfig
Quoted 10 lines
drivers/soc/qcom/Kconfig +10
-0+ 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) { + ret = -ENOMEM; + return ret;
same here
Quoted 59 lines
drivers/soc/qcom/Kconfig +59
-0+ } + + 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); + + eud_private = chip; + eud_ready = true; + + /* Enable EUD */ + if (enable) + enable_eud(eud_private); + + return 0; +} + +static int msm_eud_remove(struct platform_device *pdev) +{ + struct eud_chip *chip = platform_get_drvdata(pdev); + + if (enable) + disable_eud(eud_private); + device_init_wakeup(&pdev->dev, false); + disable_irq_wake(chip->eud_irq); + platform_set_drvdata(pdev, NULL); + + 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");
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Greg KH
maintainerRe: [PATCH v2 2/2] Embedded USB Debugger (EUD) driver
Quoted 17 lines · Manu Gautam
Hi,
Quoted 13 lines · Prakruthi Deepak Heragu
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.
Signed-off-by Satya Durga Srinivasu Prabhala <[email protected]>Signed-off-by Prakruthi Deepak Heragu <[email protected]>--- drivers/soc/qcom/Kconfig | 12 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 338 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 drivers/soc/qcom/eud.c
Quoted 7 lines
Quoted 1 line
drivers/soc/qcom/Kconfig +0
-0[snip]
drivers/soc/qcom/Kconfig
Quoted 3 lines
drivers/soc/qcom/Kconfig +3
-0+ +#define EUD_ENABLE_CMD 1 +#define EUD_DISABLE_CMD 0Why not use module param as boolean? I mean zero to disable and non-zero to enable?
Never use module parameters on new code, as it's really hard to ever change them. It also doesn't work for multiple devices of the same type.
thanks,
greg k-h
Re: [PATCH v2 2/2] Embedded USB Debugger (EUD) driver
Quoted 26 lines · Greg KH
Quoted 17 lines · Manu Gautam
Hi,
Quoted 13 lines · Prakruthi Deepak Heragu
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.
Signed-off-by Satya Durga Srinivasu Prabhala <[email protected]>Signed-off-by Prakruthi Deepak Heragu <[email protected]>--- drivers/soc/qcom/Kconfig | 12 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 338 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 drivers/soc/qcom/eud.cQuoted 7 lines
Quoted 1 line
drivers/soc/qcom/Kconfig +0
-0[snip]
drivers/soc/qcom/Kconfig
Quoted 3 lines
drivers/soc/qcom/Kconfig +3
-0+ +#define EUD_ENABLE_CMD 1 +#define EUD_DISABLE_CMD 0Why not use module param as boolean? I mean zero to disable and non-zero to enable?
Quoted 4 lines
Never use module parameters on new code, as it's really hard to ever change them. It also doesn't work for multiple devices of the same type.
If not for module_param, do you suggest we use sysfs? module_param also provides a facility to control these parameters through command line too. If we can't use module_param, do you expect us to use __setup(..) API?
Quoted 3 lines
thanks,
greg k-h
Regards,
Prakruthi Deepak Heragu
Greg KH
maintainerRe: [PATCH v2 2/2] Embedded USB Debugger (EUD) driver
Quoted 28 lines · On Tue, Sep 11, 2018 at 01:40:19PM -0700, [email protected] wrote:
Quoted 27 lines · Greg KH
Quoted 25 lines · Manu Gautam
Hi,
Quoted 14 lines · Prakruthi Deepak Heragu
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.
Signed-off-by Satya Durga Srinivasu Prabhala <[email protected]>Signed-off-by Prakruthi Deepak Heragu <[email protected]>--- drivers/soc/qcom/Kconfig | 12 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/eud.c | 338 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 drivers/soc/qcom/eud.cdrivers/soc/qcom/Kconfig +0
-0[snip]
drivers/soc/qcom/Kconfig
Quoted 3 lines
drivers/soc/qcom/Kconfig +3
-0+ +#define EUD_ENABLE_CMD 1 +#define EUD_DISABLE_CMD 0Why not use module param as boolean? I mean zero to disable and non-zero to enable?
Quoted 5 lines
Quoted 4 lines
Never use module parameters on new code, as it's really hard to ever change them. It also doesn't work for multiple devices of the same type.
If not for
module_param, do you suggest we use sysfs?
It depends on what you want to control.
Quoted 3 lines
module_paramalso provides a facility to control these parameters through command line too. If we can't usemodule_param, do you expect us to use__setup(..)API?
Again, it depends on what you are wanting to handle here. module parameters and setup variables are all "global" in that they affect all devices of the same type. sysfs can be "per device" which is almost always what you want to have happen.
But don't use any of those for things that should be "automatic" in that the driver should figure out what needs to be done here and not need any external configuration. That's the best way of all to handle this.
thanks,
greg k-h