Re: [PATCH] staging: gdm724x: Add parenthesis to Macro arguments
Hi Greg, thanks for replying. On 03/04/2019 01:26, Greg KH wrote: > On Tue, Apr 02, 2019 at 10:04:05PM -0300, Andre Dainez wrote: >> Fix checkpatch errors: >> >> CHECK: Macro argument 'len' may be better as '(len)' to avoid precedence >> issues >> CHECK: Macro argument 'nlh' may be better as '(nlh)' to avoid precedence >> issues >> >> Signed-off-by: Andre Dainez >> --- >> drivers/staging/gdm724x/netlink_k.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/staging/gdm724x/netlink_k.c >> b/drivers/staging/gdm724x/netlink_k.c >> index 92440c3..36d88f4 100644 >> --- a/drivers/staging/gdm724x/netlink_k.c >> +++ b/drivers/staging/gdm724x/netlink_k.c >> @@ -19,8 +19,8 @@ static DEFINE_MUTEX(netlink_mutex); >> #define ND_NLMSG_SPACE(len) (NLMSG_SPACE(len) + ND_IFINDEX_LEN) >> #define ND_NLMSG_DATA(nlh) ((void *)((char *)NLMSG_DATA(nlh) + \ >>ND_IFINDEX_LEN)) >> -#define ND_NLMSG_S_LEN(len) (len + ND_IFINDEX_LEN) >> -#define ND_NLMSG_R_LEN(nlh) (nlh->nlmsg_len - ND_IFINDEX_LEN) >> +#define ND_NLMSG_S_LEN(len) ((len) + ND_IFINDEX_LEN) > > This makes sense, but: > >> +#define ND_NLMSG_R_LEN(nlh) ((nlh)->nlmsg_len - ND_IFINDEX_LEN) > > That does not, correct? > Could you please clarify why this doesn't make sense? If, for some reason I calculate by hand the pointer address and call this macro like: ND_NLMSG_R_LEN(nlh + sizeof(*nlh)), then it would expand like nlh + sizeof(*nlh)->nlmsg_len - ND_IFINDEX_LEN which looks wrong in my pov, no? Please, let me know if I misunderstood anything. Thanks ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: gdm724x: Add parenthesis to Macro arguments
Fix checkpatch errors: CHECK: Macro argument 'len' may be better as '(len)' to avoid precedence issues CHECK: Macro argument 'nlh' may be better as '(nlh)' to avoid precedence issues Signed-off-by: Andre Dainez --- drivers/staging/gdm724x/netlink_k.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gdm724x/netlink_k.c b/drivers/staging/gdm724x/netlink_k.c index 92440c3..36d88f4 100644 --- a/drivers/staging/gdm724x/netlink_k.c +++ b/drivers/staging/gdm724x/netlink_k.c @@ -19,8 +19,8 @@ static DEFINE_MUTEX(netlink_mutex); #define ND_NLMSG_SPACE(len)(NLMSG_SPACE(len) + ND_IFINDEX_LEN) #define ND_NLMSG_DATA(nlh) ((void *)((char *)NLMSG_DATA(nlh) + \ ND_IFINDEX_LEN)) -#define ND_NLMSG_S_LEN(len)(len + ND_IFINDEX_LEN) -#define ND_NLMSG_R_LEN(nlh)(nlh->nlmsg_len - ND_IFINDEX_LEN) +#define ND_NLMSG_S_LEN(len)((len) + ND_IFINDEX_LEN) +#define ND_NLMSG_R_LEN(nlh)((nlh)->nlmsg_len - ND_IFINDEX_LEN) #define ND_NLMSG_IFIDX(nlh)NLMSG_DATA(nlh) #define ND_MAX_MSG_LEN (1024 * 32) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: fix typo s/informations/information
Fix checkpatch check: 'informations' may be misspelled - perhaps
'information'? in rtw_mlme_ext.c:1151.
Signed-off-by: Andre Pinto
---
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 36841d20c..04897cd48 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -1151,7 +1151,7 @@ static void issue_assocreq(struct adapter *padapter)
if (!padapter->registrypriv.wifi_spec) {
/* Commented by Kurt 20110629 */
/* In some older APs, WPS handshake */
- /* would be fail if we append vender
extensions informations to AP */
+ /* would be fail if we append vender
extensions information to AP */
if (!memcmp(pIE->data, WPS_OUI, 4))
pIE->Length = 14;
}
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: dwc2: set up all module params
On Mon, Nov 25, 2013 at 10:02:30PM -0700, Stephen Warren wrote:
> The DWC2 USB controller in the BCM2835 (Raspberry Pi) needs some non-
> default parameters. Select these based on the compatible value from the
> DT node. For all other HW, fall back to the default parameters currently
> in use.
>
> The values in params_bcm2835[] were posted to the mailing list by Paul
> quite some time ago. I made a couple of minor modifications since then;
> to set ahbcfg instead of ahb_single, and to set uframe_sched.
>
> Signed-off-by: Stephen Warren
> ---
> Documentation/devicetree/bindings/staging/dwc2.txt |4 +-
> drivers/staging/dwc2/platform.c| 56
>
> 2 files changed, 50 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
> b/Documentation/devicetree/bindings/staging/dwc2.txt
> index 1a1b7cf..8b60e90 100644
> --- a/Documentation/devicetree/bindings/staging/dwc2.txt
> +++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> @@ -2,7 +2,9 @@ Platform DesignWare HS OTG USB 2.0 controller
> -
>
> Required properties:
> -- compatible : "snps,dwc2"
> +- compatible : One of:
> + - brcm,bcm2835-usb: The DWC2 USB controller instance in the BCM2835 SoC.
> + - snps,dwc2: A generic DWC2 USB controller with default parameters.
> - reg : Should contain 1 register range (address and length)
> - interrupts : Should contain 1 interrupt
>
> diff --git a/drivers/staging/dwc2/platform.c b/drivers/staging/dwc2/platform.c
> index 4d9fac0..20586dc 100644
> --- a/drivers/staging/dwc2/platform.c
> +++ b/drivers/staging/dwc2/platform.c
> @@ -39,6 +39,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "core.h"
> @@ -46,6 +47,33 @@
>
> static const char dwc2_driver_name[] = "dwc2";
>
> +static const struct dwc2_core_params params_bcm2835 = {
> + .otg_cap= 0,/* HNP/SRP capable */
> + .otg_ver= 0,/* 1.3 */
> + .dma_enable = 1,
> + .dma_desc_enable= 0,
> + .speed = 0,/* High Speed */
> + .enable_dynamic_fifo= 1,
> + .en_multiple_tx_fifo= 1,
> + .host_rx_fifo_size = 774, /* 774 DWORDs */
> + .host_nperio_tx_fifo_size = 256, /* 256 DWORDs */
> + .host_perio_tx_fifo_size= 512, /* 512 DWORDs */
> + .max_transfer_size = 65535,
> + .max_packet_count = 511,
> + .host_channels = 8,
> + .phy_type = 1,/* UTMI */
> + .phy_utmi_width = 8,/* 8 bits */
> + .phy_ulpi_ddr = 0,/* Single */
> + .phy_ulpi_ext_vbus = 0,
> + .i2c_enable = 0,
> + .ulpi_fs_ls = 0,
> + .host_support_fs_ls_low_power = 0,
> + .host_ls_low_power_phy_clk = 0,/* 48 MHz */
> + .ts_dline = 0,
> + .reload_ctl = 0,
> + .ahbcfg = 0x10,
> +};
> +
> /**
> * dwc2_driver_remove() - Called when the DWC_otg core is unregistered with
> the
> * DWC_otg driver
> @@ -66,6 +94,13 @@ static int dwc2_driver_remove(struct platform_device *dev)
> return 0;
> }
>
> +static const struct of_device_id dwc2_of_match_table[] = {
> + { .compatible = "brcm,bcm2835-usb", .data = ¶ms_bcm2835 },
> + { .compatible = "snps,dwc2", .data = NULL },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
> +
> /**
> * dwc2_driver_probe() - Called when the DWC_otg core is bound to the DWC_otg
> * driver
> @@ -80,14 +115,23 @@ static int dwc2_driver_remove(struct platform_device
> *dev)
> */
> static int dwc2_driver_probe(struct platform_device *dev)
> {
> + const struct of_device_id *match;
> + const struct dwc2_core_params *params;
> + struct dwc2_core_params defparams;
> struct dwc2_hsotg *hsotg;
> struct resource *res;
> int retval;
> int irq;
> - struct dwc2_core_params params;
> +
> + match = of_match_device(dwc2_of_match_table, &dev->dev);
> + if (match)
> + params = match->data;
> + else {
> + dwc2_set_all_params(&defparams, -1);
> + params = &defparams;
> + }
If that matches "snps,dwc2", params will be NULL instead of the default set,
which I guess was not your intention?
>
> /* Default all params to autodetect */
> - dwc2_set_all_params(¶ms, -1);
The comment can be moved too.
>
> hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
> if (!hsotg)
> @@ -118,7 +162,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
> dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n",
> (unsigned long)res->start, hsotg->regs);
>
> -
Re: [PATCH v4] Move DWC2 driver out of staging
Hi,
On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote:
> The DWC2 driver should now be in good enough shape to move out of
> staging. I have stress tested it overnight on RPI running mass
> storage and Ethernet transfers in parallel, and for several days
> on our proprietary PCI-based platform.
>
> Signed-off-by: Paul Zimmerman
> ---
> v4: Also change directory path in MAINTAINERS
>
> Greg,
> I believe I have addressed all of the feedback since I last
> submitted this. Is there still time to do this before you
> close your trees?
>
> MAINTAINERS | 2 +-
> drivers/staging/Kconfig | 2 --
> drivers/staging/Makefile | 1 -
> drivers/staging/dwc2/TODO | 33
> ---
> drivers/usb/Kconfig | 2 ++
> drivers/usb/Makefile | 1 +
> drivers/{staging => usb}/dwc2/Kconfig | 0
> drivers/{staging => usb}/dwc2/Makefile| 0
> drivers/{staging => usb}/dwc2/core.c | 0
> drivers/{staging => usb}/dwc2/core.h | 0
> drivers/{staging => usb}/dwc2/core_intr.c | 0
> drivers/{staging => usb}/dwc2/hcd.c | 0
> drivers/{staging => usb}/dwc2/hcd.h | 0
> drivers/{staging => usb}/dwc2/hcd_ddma.c | 0
> drivers/{staging => usb}/dwc2/hcd_intr.c | 0
> drivers/{staging => usb}/dwc2/hcd_queue.c | 0
> drivers/{staging => usb}/dwc2/hw.h| 0
> drivers/{staging => usb}/dwc2/pci.c | 0
> drivers/{staging => usb}/dwc2/platform.c | 0
> 19 files changed, 4 insertions(+), 37 deletions(-)
> delete mode 100644 drivers/staging/dwc2/TODO
> rename drivers/{staging => usb}/dwc2/Kconfig (100%)
> rename drivers/{staging => usb}/dwc2/Makefile (100%)
> rename drivers/{staging => usb}/dwc2/core.c (100%)
> rename drivers/{staging => usb}/dwc2/core.h (100%)
> rename drivers/{staging => usb}/dwc2/core_intr.c (100%)
> rename drivers/{staging => usb}/dwc2/hcd.c (100%)
> rename drivers/{staging => usb}/dwc2/hcd.h (100%)
> rename drivers/{staging => usb}/dwc2/hcd_ddma.c (100%)
> rename drivers/{staging => usb}/dwc2/hcd_intr.c (100%)
> rename drivers/{staging => usb}/dwc2/hcd_queue.c (100%)
> rename drivers/{staging => usb}/dwc2/hw.h (100%)
> rename drivers/{staging => usb}/dwc2/pci.c (100%)
> rename drivers/{staging => usb}/dwc2/platform.c (100%)
this looks just fine, but for whatever reason it breaks sdhci on my rpi.
With today's Linus' master the dwc2 controller seems to initialize fine,
but I get this upon boot:
[1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12
[1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12
That is:
struct sdhci_host *sdhci_pltfm_init(struct platform_device
*pdev,
const struct
sdhci_pltfm_data *pdata,
size_t priv_size)
{
...
iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!iomem) {
ret = -ENOMEM;
goto err;
}
...
So far it's 100% reproducible. No further infos since my root device
went away. Same behavior with bcm2835_defconfig.
Bisecting points to this commit, and if I move this driver back to
staging (revert 197ba5f406cc) usb and sdhci are both working properly.
Without the revert, this patch on top...
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index d01d0d3..eaba547 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -124,6 +124,9 @@ static int dwc2_driver_probe(struct platform_device *dev)
int retval;
int irq;
+ if (usb_disabled())
+ return -ENODEV;
+
match = of_match_device(dwc2_of_match_table, &dev->dev);
if (match && match->data) {
params = match->data;
...and "nousb" in the cmdline (with crashes without the patch), sdhci works
again. I don't see any obvious clues, any idea what's going on?
Regards,
Andre
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] Move DWC2 driver out of staging
On Fri, Jan 31, 2014 at 12:15:26PM -0600, Felipe Balbi wrote:
> Hi,
>
> On Fri, Jan 31, 2014 at 07:12:36PM +0100, Andre Heider wrote:
> > Hi,
> >
> > On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote:
> > > The DWC2 driver should now be in good enough shape to move out of
> > > staging. I have stress tested it overnight on RPI running mass
> > > storage and Ethernet transfers in parallel, and for several days
> > > on our proprietary PCI-based platform.
> > >
> > > Signed-off-by: Paul Zimmerman
> > > ---
> > > v4: Also change directory path in MAINTAINERS
> > >
> > > Greg,
> > > I believe I have addressed all of the feedback since I last
> > > submitted this. Is there still time to do this before you
> > > close your trees?
> > >
> > > MAINTAINERS | 2 +-
> > > drivers/staging/Kconfig | 2 --
> > > drivers/staging/Makefile | 1 -
> > > drivers/staging/dwc2/TODO | 33
> > > ---
> > > drivers/usb/Kconfig | 2 ++
> > > drivers/usb/Makefile | 1 +
> > > drivers/{staging => usb}/dwc2/Kconfig | 0
> > > drivers/{staging => usb}/dwc2/Makefile| 0
> > > drivers/{staging => usb}/dwc2/core.c | 0
> > > drivers/{staging => usb}/dwc2/core.h | 0
> > > drivers/{staging => usb}/dwc2/core_intr.c | 0
> > > drivers/{staging => usb}/dwc2/hcd.c | 0
> > > drivers/{staging => usb}/dwc2/hcd.h | 0
> > > drivers/{staging => usb}/dwc2/hcd_ddma.c | 0
> > > drivers/{staging => usb}/dwc2/hcd_intr.c | 0
> > > drivers/{staging => usb}/dwc2/hcd_queue.c | 0
> > > drivers/{staging => usb}/dwc2/hw.h| 0
> > > drivers/{staging => usb}/dwc2/pci.c | 0
> > > drivers/{staging => usb}/dwc2/platform.c | 0
> > > 19 files changed, 4 insertions(+), 37 deletions(-)
> > > delete mode 100644 drivers/staging/dwc2/TODO
> > > rename drivers/{staging => usb}/dwc2/Kconfig (100%)
> > > rename drivers/{staging => usb}/dwc2/Makefile (100%)
> > > rename drivers/{staging => usb}/dwc2/core.c (100%)
> > > rename drivers/{staging => usb}/dwc2/core.h (100%)
> > > rename drivers/{staging => usb}/dwc2/core_intr.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd.h (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd_ddma.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd_intr.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd_queue.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hw.h (100%)
> > > rename drivers/{staging => usb}/dwc2/pci.c (100%)
> > > rename drivers/{staging => usb}/dwc2/platform.c (100%)
> >
> > this looks just fine, but for whatever reason it breaks sdhci on my rpi.
> > With today's Linus' master the dwc2 controller seems to initialize fine,
> > but I get this upon boot:
> >
> > [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12
> > [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12
> >
> > That is:
> >
> > struct sdhci_host *sdhci_pltfm_init(struct platform_device
> > *pdev,
> > const struct
> > sdhci_pltfm_data *pdata,
> > size_t priv_size)
> > {
> > ...
> >
> > iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > if (!iomem) {
> > ret = -ENOMEM;
> > goto err;
> > }
> >
> > ...
> >
> > So far it's 100% reproducible. No further infos since my root device
> > went away. Same behavior with bcm2835_defconfig.
> >
> > Bisecting points to this commit, and if I move this driver back to
> > staging (revert 197ba5f406cc) usb and sdhci are both working properly.
> >
> > Without the revert, this patch on top...
> >
> > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> > index d01d0d3..eaba547 100644
> > --- a/drivers/usb/dwc2/platform.c
> > +++ b/drivers/usb/dwc2/platform.c
> > @@ -
Re: [PATCH v4] Move DWC2 driver out of staging
On Fri, Jan 31, 2014 at 12:15:26PM -0600, Felipe Balbi wrote:
> Hi,
>
> On Fri, Jan 31, 2014 at 07:12:36PM +0100, Andre Heider wrote:
> > Hi,
> >
> > On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote:
> > > The DWC2 driver should now be in good enough shape to move out of
> > > staging. I have stress tested it overnight on RPI running mass
> > > storage and Ethernet transfers in parallel, and for several days
> > > on our proprietary PCI-based platform.
> > >
> > > Signed-off-by: Paul Zimmerman
> > > ---
> > > v4: Also change directory path in MAINTAINERS
> > >
> > > Greg,
> > > I believe I have addressed all of the feedback since I last
> > > submitted this. Is there still time to do this before you
> > > close your trees?
> > >
> > > MAINTAINERS | 2 +-
> > > drivers/staging/Kconfig | 2 --
> > > drivers/staging/Makefile | 1 -
> > > drivers/staging/dwc2/TODO | 33
> > > ---
> > > drivers/usb/Kconfig | 2 ++
> > > drivers/usb/Makefile | 1 +
> > > drivers/{staging => usb}/dwc2/Kconfig | 0
> > > drivers/{staging => usb}/dwc2/Makefile| 0
> > > drivers/{staging => usb}/dwc2/core.c | 0
> > > drivers/{staging => usb}/dwc2/core.h | 0
> > > drivers/{staging => usb}/dwc2/core_intr.c | 0
> > > drivers/{staging => usb}/dwc2/hcd.c | 0
> > > drivers/{staging => usb}/dwc2/hcd.h | 0
> > > drivers/{staging => usb}/dwc2/hcd_ddma.c | 0
> > > drivers/{staging => usb}/dwc2/hcd_intr.c | 0
> > > drivers/{staging => usb}/dwc2/hcd_queue.c | 0
> > > drivers/{staging => usb}/dwc2/hw.h| 0
> > > drivers/{staging => usb}/dwc2/pci.c | 0
> > > drivers/{staging => usb}/dwc2/platform.c | 0
> > > 19 files changed, 4 insertions(+), 37 deletions(-)
> > > delete mode 100644 drivers/staging/dwc2/TODO
> > > rename drivers/{staging => usb}/dwc2/Kconfig (100%)
> > > rename drivers/{staging => usb}/dwc2/Makefile (100%)
> > > rename drivers/{staging => usb}/dwc2/core.c (100%)
> > > rename drivers/{staging => usb}/dwc2/core.h (100%)
> > > rename drivers/{staging => usb}/dwc2/core_intr.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd.h (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd_ddma.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd_intr.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hcd_queue.c (100%)
> > > rename drivers/{staging => usb}/dwc2/hw.h (100%)
> > > rename drivers/{staging => usb}/dwc2/pci.c (100%)
> > > rename drivers/{staging => usb}/dwc2/platform.c (100%)
> >
> > this looks just fine, but for whatever reason it breaks sdhci on my rpi.
> > With today's Linus' master the dwc2 controller seems to initialize fine,
> > but I get this upon boot:
> >
> > [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12
> > [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12
> >
> > That is:
> >
> > struct sdhci_host *sdhci_pltfm_init(struct platform_device
> > *pdev,
> > const struct
> > sdhci_pltfm_data *pdata,
> > size_t priv_size)
> > {
> > ...
> >
> > iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > if (!iomem) {
> > ret = -ENOMEM;
> > goto err;
> > }
> >
> > ...
> >
> > So far it's 100% reproducible. No further infos since my root device
> > went away. Same behavior with bcm2835_defconfig.
> >
> > Bisecting points to this commit, and if I move this driver back to
> > staging (revert 197ba5f406cc) usb and sdhci are both working properly.
> >
> > Without the revert, this patch on top...
> >
> > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> > index d01d0d3..eaba547 100644
> > --- a/drivers/usb/dwc2/platform.c
> > +++ b/drivers/usb/dwc2/platform.c
> > @@
Re: [PATCH v4] Move DWC2 driver out of staging
On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote:
> On 01/31/2014 11:12 AM, Andre Heider wrote:
> > On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote:
> >> The DWC2 driver should now be in good enough shape to move out of
> >> staging. I have stress tested it overnight on RPI running mass
> >> storage and Ethernet transfers in parallel, and for several days
> >> on our proprietary PCI-based platform.
> ...
> > this looks just fine, but for whatever reason it breaks sdhci on my rpi.
> > With today's Linus' master the dwc2 controller seems to initialize fine,
> > but I get this upon boot:
> >
> > [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12
> > [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12
> >
> > That is:
> >
> > struct sdhci_host *sdhci_pltfm_init(struct platform_device
> > *pdev,
> > const struct
> > sdhci_pltfm_data *pdata,
> > size_t priv_size)
> > {
> > ...
> >
> > iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > if (!iomem) {
> > ret = -ENOMEM;
> > goto err;
> > }
>
> This is due to the following code:
>
> static void _dwc2_hcd_endpoint_reset(struct usb_hcd *hcd,
>struct usb_host_endpoint *ep)
> {
> struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> ...
> struct usb_device *udev;
> ...
> udev = to_usb_device(hsotg->dev);
> ...
> usb_settoggle(udev, epnum, is_out, 0);
> if (is_control)
> usb_settoggle(udev, epnum, !is_out, 0);
>
> The problem is that hsotg->dev is assigned as follows:
>
> static int dwc2_driver_probe(struct platform_device *dev)
> ...
> hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
> ...
> hsotg->dev = &dev->dev;
>
> As such, it's not legal to call to_usb_device() on it.
>
> What ends up happening, simply due to memory allocation order, is that
> the memory writes inside usb_settoggle() end up setting the SDHCI struct
> platform_device's num_resources to 0, so that it's call to
> platform_get_resource() fails.
>
> With the DWC2 move patch reverted, some other random piece of memory is
> being corrupted, which just happens not to cause any visible problem.
> Likely it's some other struct platform_device that's already had its
> resources read by the time DWC2 probes and corrupts them.
>
> (Yes, this was hard to find!)
Nice work, but how did you pinpoint this? Am I missing some option/tool
or did I just not stare for long enough?
> I honestly can't see how to solve this myself, since the whole DWC2
> driver doesn't seem to have a struct usb_device * hanging around that we
> can stash somewhere for it to look up later. Perhaps someone more
> familiar with the USB stack can help with that.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] Move DWC2 driver out of staging
On Mon, Feb 03, 2014 at 08:51:48PM +, Paul Zimmerman wrote: > > From: Paul Zimmerman > > Sent: Monday, February 03, 2014 9:36 AM > > > >> From: Stephen Warren [mailto:swar...@wwwdotorg.org] > >> Sent: Saturday, February 01, 2014 7:44 PM > >> > >> On 02/01/2014 03:00 AM, Andre Heider wrote: > >>> On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote: > >>>> On 01/31/2014 11:12 AM, Andre Heider wrote: > >>>>> On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: > >>>>>> The DWC2 driver should now be in good enough shape to move out of > >>>>>> staging. I have stress tested it overnight on RPI running mass > >>>>>> storage and Ethernet transfers in parallel, and for several days > >>>>>> on our proprietary PCI-based platform. > >>>> ... > >>>>> this looks just fine, but for whatever reason it breaks sdhci on my rpi. > >>>>> With today's Linus' master the dwc2 controller seems to initialize fine, > >>>>> but I get this upon boot: > >>>>> > >>>>> [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12 > >>>>> [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error > >>>>> -12 > >> ... > >>>> This is due to the following code: > >> ... > >>>> What ends up happening, simply due to memory allocation order, is that > >>>> the memory writes inside usb_settoggle() end up setting the SDHCI struct > >>>> platform_device's num_resources to 0, so that it's call to > >>>> platform_get_resource() fails. > >>>> > >>>> With the DWC2 move patch reverted, some other random piece of memory is > >>>> being corrupted, which just happens not to cause any visible problem. > >>>> Likely it's some other struct platform_device that's already had its > >>>> resources read by the time DWC2 probes and corrupts them. > >>>> > >>>> (Yes, this was hard to find!) > >>> > >>> Nice work, but how did you pinpoint this? Am I missing some option/tool > >>> or did I just not stare for long enough? > >> > >> Well, there was a clear place where an issue was present; the resource > >> lookup in sdhci_pltfm_init() was failing, so I put a bunch of printfs > >> into that function to dump out the data platform_get_resource() used. > >> This clearly pointed at num_resources==0 being the problem. Next, I > >> dumped the same data from the code in drivers/of that sets it up, and it > >> was OK there, so I knew it was getting over-written somewhere. I then > >> basically added hundreds of calls to the same data dumping function > >> throughout kernel functions like really_probe() to track down the > >> location of the problem. Luckily, the behaviour was stable, so I wasn't > >> chasing a race/timing condition. Eventually I narrowed the window to the > >> few lines of code I mentioned in _dwc2_hcd_endpoint_reset(). It would > >> have been much harder if it was e.g. the USB HW DMAing to memory that > >> caused the corruption, so I was lucky:-) > > > > Nice work Stephen, thanks. I will try to come up with a patch to fix this > > ASAP, along the lines of what Alan suggested. > > Stephen, Andre, > > Can you test the attached patch, please? It works for my on the Synopsys > PCIe-based FPGA board. Unfortunately my RPI board is currently broken, > so I am unable to test it there to verify it actually fixes the problem > you are seeing. > > The dwc2 driver doesn't use the usb_device toggle bits anywhere else, > so the quickest fix is to just remove the problematic code from > _dwc2_hcd_endpoint_reset(). > > If you give me your tested-bys, I will submit this as a proper patch > to Greg. I'll give it a spin this evening, thanks. Is that really just redundant code or could this removal have side effects? Should I look out for anything specific? Oh, and I'm not sure if I poked the right spot with the "nousb" fix, but I'll send that out as well. Regards, Andre ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] Move DWC2 driver out of staging
On Mon, Feb 03, 2014 at 08:51:48PM +, Paul Zimmerman wrote: > > From: Paul Zimmerman > > Sent: Monday, February 03, 2014 9:36 AM > > > >> From: Stephen Warren [mailto:swar...@wwwdotorg.org] > >> Sent: Saturday, February 01, 2014 7:44 PM > >> > >> On 02/01/2014 03:00 AM, Andre Heider wrote: > >>> On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote: > >>>> On 01/31/2014 11:12 AM, Andre Heider wrote: > >>>>> On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: > >>>>>> The DWC2 driver should now be in good enough shape to move out of > >>>>>> staging. I have stress tested it overnight on RPI running mass > >>>>>> storage and Ethernet transfers in parallel, and for several days > >>>>>> on our proprietary PCI-based platform. > >>>> ... > >>>>> this looks just fine, but for whatever reason it breaks sdhci on my rpi. > >>>>> With today's Linus' master the dwc2 controller seems to initialize fine, > >>>>> but I get this upon boot: > >>>>> > >>>>> [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12 > >>>>> [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error > >>>>> -12 > >> ... > >>>> This is due to the following code: > >> ... > >>>> What ends up happening, simply due to memory allocation order, is that > >>>> the memory writes inside usb_settoggle() end up setting the SDHCI struct > >>>> platform_device's num_resources to 0, so that it's call to > >>>> platform_get_resource() fails. > >>>> > >>>> With the DWC2 move patch reverted, some other random piece of memory is > >>>> being corrupted, which just happens not to cause any visible problem. > >>>> Likely it's some other struct platform_device that's already had its > >>>> resources read by the time DWC2 probes and corrupts them. > >>>> > >>>> (Yes, this was hard to find!) > >>> > >>> Nice work, but how did you pinpoint this? Am I missing some option/tool > >>> or did I just not stare for long enough? > >> > >> Well, there was a clear place where an issue was present; the resource > >> lookup in sdhci_pltfm_init() was failing, so I put a bunch of printfs > >> into that function to dump out the data platform_get_resource() used. > >> This clearly pointed at num_resources==0 being the problem. Next, I > >> dumped the same data from the code in drivers/of that sets it up, and it > >> was OK there, so I knew it was getting over-written somewhere. I then > >> basically added hundreds of calls to the same data dumping function > >> throughout kernel functions like really_probe() to track down the > >> location of the problem. Luckily, the behaviour was stable, so I wasn't > >> chasing a race/timing condition. Eventually I narrowed the window to the > >> few lines of code I mentioned in _dwc2_hcd_endpoint_reset(). It would > >> have been much harder if it was e.g. the USB HW DMAing to memory that > >> caused the corruption, so I was lucky:-) > > > > Nice work Stephen, thanks. I will try to come up with a patch to fix this > > ASAP, along the lines of what Alan suggested. > > Stephen, Andre, > > Can you test the attached patch, please? It works for my on the Synopsys > PCIe-based FPGA board. Unfortunately my RPI board is currently broken, > so I am unable to test it there to verify it actually fixes the problem > you are seeing. > > The dwc2 driver doesn't use the usb_device toggle bits anywhere else, > so the quickest fix is to just remove the problematic code from > _dwc2_hcd_endpoint_reset(). > > If you give me your tested-bys, I will submit this as a proper patch > to Greg. LGTM, sdhci works again and there're no glaring USB issues with lan, hid nor mass storage: Tested-by: Andre Heider Thanks Paul, Andre ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
