[U-Boot] [PATCH 1/2] usb: dwc2: correctly handle binding for g-tx-fifo-size
Manage g-tx-fifo-size as a array as specify in the binding.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi | 4 ---- drivers/usb/gadget/dwc2_udc_otg.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi index 5b19e44..994092a 100644 --- a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi @@ -56,10 +56,6 @@ }; };
-&usbotg_hs { - g-tx-fifo-size = <576>; -}; - &v3v3 { regulator-always-on; }; diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 494ab53..7e6b5fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1039,6 +1039,8 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct udevice *dev) int node = dev_of_offset(dev); ulong drvdata; void (*set_params)(struct dwc2_plat_otg_data *data); + u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS]; + int ret, i;
if (usb_get_dr_mode(node) != USB_DR_MODE_PERIPHERAL) { dev_dbg(dev, "Invalid mode\n"); @@ -1050,7 +1052,20 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct udevice *dev) platdata->rx_fifo_sz = dev_read_u32_default(dev, "g-rx-fifo-size", 0); platdata->np_tx_fifo_sz = dev_read_u32_default(dev, "g-np-tx-fifo-size", 0); - platdata->tx_fifo_sz = dev_read_u32_default(dev, "g-tx-fifo-size", 0); + + platdata->tx_fifo_sz_nb = + dev_read_size(dev, "g-tx-fifo-size") / sizeof(u32); + if (platdata->tx_fifo_sz_nb > DWC2_MAX_HW_ENDPOINTS) + platdata->tx_fifo_sz_nb = DWC2_MAX_HW_ENDPOINTS; + if (platdata->tx_fifo_sz_nb) { + ret = dev_read_u32_array(dev, "g-tx-fifo-size", + tx_fifo_sz_array, + platdata->tx_fifo_sz_nb); + if (ret) + return ret; + for (i = 0; i < platdata->tx_fifo_sz_nb; i++) + platdata->tx_fifo_sz_array[i] = tx_fifo_sz_array[i]; + }
platdata->force_b_session_valid = dev_read_bool(dev, "u-boot,force-b-session-valid");
Allow device mode in DWC2 driver when device tree select the dr_mode "peripheral" or "otg".
The device mode is not allowed when dr_mode = "host" in device tree.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
drivers/usb/gadget/dwc2_udc_otg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 7e6b5fc..895f8c9 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1042,7 +1042,8 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct udevice *dev) u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS]; int ret, i;
- if (usb_get_dr_mode(node) != USB_DR_MODE_PERIPHERAL) { + if (usb_get_dr_mode(node) != USB_DR_MODE_PERIPHERAL && + usb_get_dr_mode(node) != USB_DR_MODE_OTG) { dev_dbg(dev, "Invalid mode\n"); return -ENODEV; }
On 6/14/19 1:08 PM, Patrick Delaunay wrote:
Manage g-tx-fifo-size as a array as specify in the binding.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi | 4 ---- drivers/usb/gadget/dwc2_udc_otg.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi index 5b19e44..994092a 100644 --- a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi @@ -56,10 +56,6 @@ }; };
-&usbotg_hs {
- g-tx-fifo-size = <576>;
Should this really be in this patch ?
-};
&v3v3 { regulator-always-on; }; diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 494ab53..7e6b5fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1039,6 +1039,8 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct udevice *dev) int node = dev_of_offset(dev); ulong drvdata; void (*set_params)(struct dwc2_plat_otg_data *data);
- u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
Can't you just read directly into platdata->tx_fifo_sz_array[] below, and thus drop this temporary variable ?
btw is this fix for current release or next ?
int ret, i;
if (usb_get_dr_mode(node) != USB_DR_MODE_PERIPHERAL) { dev_dbg(dev, "Invalid mode\n");
@@ -1050,7 +1052,20 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct udevice *dev) platdata->rx_fifo_sz = dev_read_u32_default(dev, "g-rx-fifo-size", 0); platdata->np_tx_fifo_sz = dev_read_u32_default(dev, "g-np-tx-fifo-size", 0);
- platdata->tx_fifo_sz = dev_read_u32_default(dev, "g-tx-fifo-size", 0);
platdata->tx_fifo_sz_nb =
dev_read_size(dev, "g-tx-fifo-size") / sizeof(u32);if (platdata->tx_fifo_sz_nb > DWC2_MAX_HW_ENDPOINTS)
platdata->tx_fifo_sz_nb = DWC2_MAX_HW_ENDPOINTS;if (platdata->tx_fifo_sz_nb) {
ret = dev_read_u32_array(dev, "g-tx-fifo-size",tx_fifo_sz_array,platdata->tx_fifo_sz_nb);if (ret)return ret;for (i = 0; i < platdata->tx_fifo_sz_nb; i++)platdata->tx_fifo_sz_array[i] = tx_fifo_sz_array[i];}
platdata->force_b_session_valid = dev_read_bool(dev, "u-boot,force-b-session-valid");
Hi Marek,
From: Marek Vasut marex@denx.de Sent: lundi 17 juin 2019 17:54
On 6/14/19 1:08 PM, Patrick Delaunay wrote:
Manage g-tx-fifo-size as a array as specify in the binding.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi | 4 ---- drivers/usb/gadget/dwc2_udc_otg.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi index 5b19e44..994092a 100644 --- a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi @@ -56,10 +56,6 @@ }; };
-&usbotg_hs {
- g-tx-fifo-size = <576>;
Should this really be in this patch ?
As I change the binding parsing, the stm32mp1 will don't work without this patch. I make a commun patch only to allow bisec, but I can split the serie with 2 patches.
-};
&v3v3 { regulator-always-on; }; diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 494ab53..7e6b5fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1039,6 +1039,8 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct
udevice *dev)
int node = dev_of_offset(dev); ulong drvdata; void (*set_params)(struct dwc2_plat_otg_data *data);
- u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
Can't you just read directly into platdata->tx_fifo_sz_array[] below, and thus drop this temporary variable ?
It was the case in in my first internal version.
if (platdata->tx_fifo_sz_nb) { ret = dev_read_u32_array(dev, "g-tx-fifo-size", &platdata->tx_fifo_sz_array, platdata->tx_fifo_sz_nb); if (ret) return ret; }
And I add it to avoid the warning / potential issue:
/local/home/frq07632/views/u-boot/u-boot/drivers/usb/gadget/dwc2_udc_otg.c:1062:7: warning: passing argument 3 of ‘dev_read_u32_array’ from incompatible pointer type [-Wincompatible-pointer-types] &platdata->tx_fifo_sz_array, ^~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /local/home/frq07632/views/u-boot/u-boot/include/dm.h:12, from /local/home/frq07632/views/u-boot/u-boot/drivers/usb/gadget/dwc2_udc_otg.c:22: /local/home/frq07632/views/u-boot/u-boot/include/dm/read.h:710:15: note: expected ‘u32 *’ {aka ‘unsigned int *’} but argument is of type ‘unsigned int (*)[16]’ u32 *out_values, size_t sz) ~~~~~^~~~~~~~~~
btw is this fix for current release or next ?
I hope it for the v2019.07 (as it is only impact the stm32mp1 arch/board). But it is not blocking.
int ret, i;
if (usb_get_dr_mode(node) != USB_DR_MODE_PERIPHERAL) { dev_dbg(dev, "Invalid mode\n");
@@ -1050,7 +1052,20 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct
udevice *dev)
platdata->rx_fifo_sz = dev_read_u32_default(dev, "g-rx-fifo-size", 0); platdata->np_tx_fifo_sz = dev_read_u32_default(dev, "g-np-tx-fifo-size", 0);
- platdata->tx_fifo_sz = dev_read_u32_default(dev, "g-tx-fifo-size", 0);
platdata->tx_fifo_sz_nb =
dev_read_size(dev, "g-tx-fifo-size") / sizeof(u32);if (platdata->tx_fifo_sz_nb > DWC2_MAX_HW_ENDPOINTS)
platdata->tx_fifo_sz_nb = DWC2_MAX_HW_ENDPOINTS;if (platdata->tx_fifo_sz_nb) {
ret = dev_read_u32_array(dev, "g-tx-fifo-size",tx_fifo_sz_array,platdata->tx_fifo_sz_nb);if (ret)return ret;for (i = 0; i < platdata->tx_fifo_sz_nb; i++)platdata->tx_fifo_sz_array[i] = tx_fifo_sz_array[i];}
platdata->force_b_session_valid = dev_read_bool(dev, "u-boot,force-b-session-valid");
-- Best regards, Marek Vasut
Regards
Patrick
On 6/18/19 9:32 AM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: lundi 17 juin 2019 17:54
On 6/14/19 1:08 PM, Patrick Delaunay wrote:
Manage g-tx-fifo-size as a array as specify in the binding.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi | 4 ---- drivers/usb/gadget/dwc2_udc_otg.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi index 5b19e44..994092a 100644 --- a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi @@ -56,10 +56,6 @@ }; };
-&usbotg_hs {
- g-tx-fifo-size = <576>;
Should this really be in this patch ?
As I change the binding parsing, the stm32mp1 will don't work without this patch. I make a commun patch only to allow bisec, but I can split the serie with 2 patches.
-};
&v3v3 { regulator-always-on; }; diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 494ab53..7e6b5fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1039,6 +1039,8 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct
udevice *dev)
int node = dev_of_offset(dev); ulong drvdata; void (*set_params)(struct dwc2_plat_otg_data *data);
- u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
Can't you just read directly into platdata->tx_fifo_sz_array[] below, and thus drop this temporary variable ?
It was the case in in my first internal version.
if (platdata->tx_fifo_sz_nb) { ret = dev_read_u32_array(dev, "g-tx-fifo-size", &platdata->tx_fifo_sz_array, platdata->tx_fifo_sz_nb); if (ret) return ret; }
And I add it to avoid the warning / potential issue:
/local/home/frq07632/views/u-boot/u-boot/drivers/usb/gadget/dwc2_udc_otg.c:1062:7: warning: passing argument 3 of ‘dev_read_u32_array’ from incompatible pointer type [-Wincompatible-pointer-types] &platdata->tx_fifo_sz_array, ^~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /local/home/frq07632/views/u-boot/u-boot/include/dm.h:12, from /local/home/frq07632/views/u-boot/u-boot/drivers/usb/gadget/dwc2_udc_otg.c:22: /local/home/frq07632/views/u-boot/u-boot/include/dm/read.h:710:15: note: expected ‘u32 *’ {aka ‘unsigned int *’} but argument is of type ‘unsigned int (*)[16]’ u32 *out_values, size_t sz) ~~~~~^~~~~~~~~~
Looks like GCC is complaining because you're passing an array of pointers to dev_read_u32_array() , instead of plain pointer .
Try
ret = dev_read_u32_array(dev, "g-tx-fifo-size", - &platdata->tx_fifo_sz_array, + platdata->tx_fifo_sz_array, platdata->tx_fifo_sz_nb);
btw is this fix for current release or next ?
I hope it for the v2019.07 (as it is only impact the stm32mp1 arch/board). But it is not blocking.
OK
Hi Marek
From: Marek Vasut marex@denx.de Sent: mardi 18 juin 2019 12:49
On 6/18/19 9:32 AM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: lundi 17 juin 2019 17:54
On 6/14/19 1:08 PM, Patrick Delaunay wrote:
Manage g-tx-fifo-size as a array as specify in the binding.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi | 4 ---- drivers/usb/gadget/dwc2_udc_otg.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi index 5b19e44..994092a 100644 --- a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi @@ -56,10 +56,6 @@ }; };
-&usbotg_hs {
- g-tx-fifo-size = <576>;
Should this really be in this patch ?
As I change the binding parsing, the stm32mp1 will don't work without this
patch.
I make a commun patch only to allow bisec, but I can split the serie with 2
patches.
-};
&v3v3 { regulator-always-on; }; diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 494ab53..7e6b5fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1039,6 +1039,8 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct
udevice *dev)
int node = dev_of_offset(dev); ulong drvdata; void (*set_params)(struct dwc2_plat_otg_data *data);
- u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
Can't you just read directly into platdata->tx_fifo_sz_array[] below, and thus drop this temporary variable ?
It was the case in in my first internal version.
if (platdata->tx_fifo_sz_nb) { ret = dev_read_u32_array(dev, "g-tx-fifo-size", &platdata->tx_fifo_sz_array, platdata->tx_fifo_sz_nb); if (ret) return ret; }
And I add it to avoid the warning / potential issue:
/local/home/frq07632/views/u-boot/u-
boot/drivers/usb/gadget/dwc2_udc_otg.c:1062:7:
warning: passing argument 3 of ‘dev_read_u32_array’ from incompatible
pointer type [-Wincompatible-pointer-types]
&platdata->tx_fifo_sz_array, ^~~~~~~~~~~~~~~~~~~~~~~~~~~In file included from /local/home/frq07632/views/u-boot/u-boot/include/dm.h:12, from /local/home/frq07632/views/u-boot/u-
boot/drivers/usb/gadget/dwc2_udc_otg.c:22:
/local/home/frq07632/views/u-boot/u-boot/include/dm/read.h:710:15: note:
expected ‘u32 *’ {aka ‘unsigned int *’} but argument is of type ‘unsigned int (*)[16]’
u32 *out_values, size_t sz) ~~~~~^~~~~~~~~~Looks like GCC is complaining because you're passing an array of pointers to dev_read_u32_array() , instead of plain pointer .
Try
ret = dev_read_u32_array(dev, "g-tx-fifo-size",
&platdata->tx_fifo_sz_array,
platdata->tx_fifo_sz_array, platdata->tx_fifo_sz_nb);
You are right, it is working I don't double check the type of the buffer :-< => I will push update in V2
btw is this fix for current release or next ?
I hope it for the v2019.07 (as it is only impact the stm32mp1 arch/board). But it is not blocking.
OK
-- Best regards, Marek Vasut
On 6/18/19 3:33 PM, Patrick DELAUNAY wrote:
Hi Marek
From: Marek Vasut marex@denx.de Sent: mardi 18 juin 2019 12:49
On 6/18/19 9:32 AM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: lundi 17 juin 2019 17:54
On 6/14/19 1:08 PM, Patrick Delaunay wrote:
Manage g-tx-fifo-size as a array as specify in the binding.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi | 4 ---- drivers/usb/gadget/dwc2_udc_otg.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi index 5b19e44..994092a 100644 --- a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi @@ -56,10 +56,6 @@ }; };
-&usbotg_hs {
- g-tx-fifo-size = <576>;
Should this really be in this patch ?
As I change the binding parsing, the stm32mp1 will don't work without this
patch.
I make a commun patch only to allow bisec, but I can split the serie with 2
patches.
-};
&v3v3 { regulator-always-on; }; diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 494ab53..7e6b5fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1039,6 +1039,8 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct
udevice *dev)
int node = dev_of_offset(dev); ulong drvdata; void (*set_params)(struct dwc2_plat_otg_data *data);
- u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
Can't you just read directly into platdata->tx_fifo_sz_array[] below, and thus drop this temporary variable ?
It was the case in in my first internal version.
if (platdata->tx_fifo_sz_nb) { ret = dev_read_u32_array(dev, "g-tx-fifo-size", &platdata->tx_fifo_sz_array, platdata->tx_fifo_sz_nb); if (ret) return ret; }
And I add it to avoid the warning / potential issue:
/local/home/frq07632/views/u-boot/u-
boot/drivers/usb/gadget/dwc2_udc_otg.c:1062:7:
warning: passing argument 3 of ‘dev_read_u32_array’ from incompatible
pointer type [-Wincompatible-pointer-types]
&platdata->tx_fifo_sz_array, ^~~~~~~~~~~~~~~~~~~~~~~~~~~In file included from /local/home/frq07632/views/u-boot/u-boot/include/dm.h:12, from /local/home/frq07632/views/u-boot/u-
boot/drivers/usb/gadget/dwc2_udc_otg.c:22:
/local/home/frq07632/views/u-boot/u-boot/include/dm/read.h:710:15: note:
expected ‘u32 *’ {aka ‘unsigned int *’} but argument is of type ‘unsigned int (*)[16]’
u32 *out_values, size_t sz) ~~~~~^~~~~~~~~~Looks like GCC is complaining because you're passing an array of pointers to dev_read_u32_array() , instead of plain pointer .
Try
ret = dev_read_u32_array(dev, "g-tx-fifo-size",
&platdata->tx_fifo_sz_array,
platdata->tx_fifo_sz_array, platdata->tx_fifo_sz_nb);You are right, it is working I don't double check the type of the buffer :-< => I will push update in V2
Thanks
participants (3)
-
Marek Vasut -
Patrick DELAUNAY -
Patrick Delaunay