| Message ID | 20230516213308.2432018-1-bhupesh.sharma@linaro.org |
|---|---|
| Headers | show |
| Series | Add Qualcomm SM6115 / SM4250 EUD dt-bindings & driver support | expand |
On Wed, May 17, 2023 at 03:03:06AM +0530, Bhupesh Sharma wrote: > Add SM6115 / SM4250 SoC EUD support in qcom_eud driver. Why is the subject line duplicated here? > On some SoCs (like the SM6115 / SM4250 SoC), the mode manager > needs to be accessed only via the secure world (through 'scm' > calls). > > Also, the enable bit inside 'tcsr_check_reg' needs to be set > first to set the eud in 'enable' mode on these SoCs. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > drivers/usb/misc/Kconfig | 1 + > drivers/usb/misc/qcom_eud.c | 69 +++++++++++++++++++++++++++++++++---- Given that you didn't cc the usb maintainer, I'm guessing you don't want this patch applied? > 2 files changed, 63 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig > index 99b15b77dfd5..fe1b5fec1dfc 100644 > --- a/drivers/usb/misc/Kconfig > +++ b/drivers/usb/misc/Kconfig > @@ -147,6 +147,7 @@ config USB_APPLEDISPLAY > config USB_QCOM_EUD > tristate "QCOM Embedded USB Debugger(EUD) Driver" > depends on ARCH_QCOM || COMPILE_TEST > + select QCOM_SCM How well is that going to work on building on non-QCOM systems? Can QCOM_SCM build if COMPILE_TEST is enabled? select is rough to get right, are you sure it's correct here? If so, some documentation in the changelog would be appreciated. > select USB_ROLE_SWITCH > help > This module enables support for Qualcomm Technologies, Inc. > diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c > index b7f13df00764..10d194604d4c 100644 > --- a/drivers/usb/misc/qcom_eud.c > +++ b/drivers/usb/misc/qcom_eud.c > @@ -5,12 +5,14 @@ > > #include <linux/bitops.h> > #include <linux/err.h> > +#include <linux/firmware/qcom/qcom_scm.h> There's no rule to keep these sorted, but it's your choice... > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/iopoll.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/slab.h> > #include <linux/sysfs.h> > @@ -22,23 +24,33 @@ > #define EUD_REG_VBUS_INT_CLR 0x0080 > #define EUD_REG_CSR_EUD_EN 0x1014 > #define EUD_REG_SW_ATTACH_DET 0x1018 > -#define EUD_REG_EUD_EN2 0x0000 > +#define EUD_REG_EUD_EN2 0x0000 Why the coding style cleanup in the same patch? Remember, changes only do one thing, and you have already listed 2 things in your commit message :( > > #define EUD_ENABLE BIT(0) > -#define EUD_INT_PET_EUD BIT(0) > +#define EUD_INT_PET_EUD BIT(0) Again, why this change? thanks, greg k-h
On Wed, 17 May 2023 at 11:08, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > On Wed, May 17, 2023 at 03:03:04AM +0530, Bhupesh Sharma wrote: > > The eud sysfs enablement path is currently mentioned in the > > Documentation as: > > /sys/bus/platform/drivers/eud/.../enable > > > > Instead it should be: > > /sys/bus/platform/drivers/qcom_eud/.../enable > > > > Fix the same. > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > I believe the path has changed during one of the EUD patch iterations. In that > case, the documentation is wrong from day one. So this patch should have the > relevant Fixes tag. > > With that, > > Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Ok, I will add a Fixes tag. Thanks, Bhupesh > > --- > > Documentation/ABI/testing/sysfs-driver-eud | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud > > index 83f3872182a4..2bab0db2d2f0 100644 > > --- a/Documentation/ABI/testing/sysfs-driver-eud > > +++ b/Documentation/ABI/testing/sysfs-driver-eud > > @@ -1,4 +1,4 @@ > > -What: /sys/bus/platform/drivers/eud/.../enable > > +What: /sys/bus/platform/drivers/qcom_eud/.../enable > > Date: February 2022 > > Contact: Souradeep Chowdhury <quic_schowdhu@quicinc.com> > > Description: > > -- > > 2.38.1 > > > > -- > மணிவண்ணன் சதாசிவம்
Changes since v4: ---------------- - v4 can be viewed here: https://lore.kernel.org/linux-arm-msm/20230505064039.1630025-1-bhupesh.sharma@linaro.org/ - Addressed Konrad's review comments regarding EUD driver code. - Also collected his R-B for [PATCH 4/5 and 5/5]. - Fixed the dt-bindings as per Krzysztof's comments. Changes since v3: ---------------- - v3 can be viewed here: https://www.spinics.net/lists/linux-arm-msm/msg137025.html - Addressed Konrad's review comments regarding mainly the driver code. Also fixed the .dtsi as per his comments. - Also collected his R-B for [PATCH 1/5]. Changes since v2: ---------------- - v2 can be viewed here: https://www.spinics.net/lists/linux-arm-msm/msg137025.html - Addressed Bjorn and Krzysztof's comments. - Added [PATCH 1/5] which fixes the 'qcom_eud' sysfs path. - Added [PATCH 5/5] to enable EUD for Qualcomm QRB4210-RB2 boards. Changes since v1: ---------------- - v1 can be viewed here: https://lore.kernel.org/linux-arm-msm/20221231130743.3285664-1-bhupesh.sharma@linaro.org - Added Krzysztof in Cc list. - Fixed the following issue reported by kernel test bot: >> ERROR: modpost: "qcom_scm_io_writel" [drivers/usb/misc/qcom_eud.ko] undefined! This series adds the dt-binding and driver support for SM6115 / SM4250 EUD (Embedded USB Debugger) block available on Qualcomm SoCs. It also enables the same for QRB4210-RB2 boards by default (the user still needs to enable the same via sysfs). The EUD is a mini-USB hub implemented on chip to support the USB-based debug and trace capabilities. EUD driver listens to events like USB attach or detach and then informs the USB about these events via ROLE-SWITCH. Bhupesh Sharma (5): usb: misc: eud: Fix eud sysfs path (use 'qcom_eud') dt-bindings: soc: qcom: eud: Add SM6115 / SM4250 support usb: misc: eud: Add driver support for SM6115 / SM4250 arm64: dts: qcom: sm6115: Add EUD dt node and dwc3 connector arm64: dts: qcom: qrb4210-rb2: Enable EUD debug peripheral Documentation/ABI/testing/sysfs-driver-eud | 2 +- .../bindings/soc/qcom/qcom,eud.yaml | 42 ++++++++++- arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 27 +++++++- arch/arm64/boot/dts/qcom/sm6115.dtsi | 50 ++++++++++++++ drivers/usb/misc/Kconfig | 1 + drivers/usb/misc/qcom_eud.c | 69 +++++++++++++++++-- 6 files changed, 179 insertions(+), 12 deletions(-)