* [PATCH] net: stmmac: implement .set_intf_mode() callback for imx8dxl
@ 2020-12-03 4:10 Joakim Zhang
2020-12-05 19:58 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Joakim Zhang @ 2020-12-03 4:10 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba; +Cc: linux-imx, netdev
From: Fugang Duan <fugang.duan@nxp.com>
Implement .set_intf_mode() callback for imx8dxl.
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-imx.c | 34 ++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index 223f69da7e95..1d0a4d73add6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -6,7 +6,9 @@
*
*/
+#include <dt-bindings/firmware/imx/rsrc.h>
#include <linux/clk.h>
+#include <linux/firmware/imx/sci.h>
#include <linux/gpio/consumer.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
@@ -86,7 +88,37 @@ imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
{
int ret = 0;
- /* TBD: depends on imx8dxl scu interfaces to be upstreamed */
+ struct imx_sc_ipc *ipc_handle;
+ int val;
+
+ ret = imx_scu_get_handle(&ipc_handle);
+ if (ret)
+ return ret;
+
+ switch (plat_dat->interface) {
+ case PHY_INTERFACE_MODE_MII:
+ val = GPR_ENET_QOS_INTF_SEL_MII;
+ break;
+ case PHY_INTERFACE_MODE_RMII:
+ val = GPR_ENET_QOS_INTF_SEL_RMII;
+ break;
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ val = GPR_ENET_QOS_INTF_SEL_RGMII;
+ break;
+ default:
+ pr_debug("imx dwmac doesn't support %d interface\n",
+ plat_dat->interface);
+ return -EINVAL;
+ }
+
+ ret = imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
+ IMX_SC_C_INTF_SEL, val >> 16);
+ ret |= imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
+ IMX_SC_C_CLK_GEN_EN, 0x1);
+
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] net: stmmac: implement .set_intf_mode() callback for imx8dxl
2020-12-03 4:10 [PATCH] net: stmmac: implement .set_intf_mode() callback for imx8dxl Joakim Zhang
@ 2020-12-05 19:58 ` Jakub Kicinski
2020-12-07 8:15 ` Joakim Zhang
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2020-12-05 19:58 UTC (permalink / raw)
To: Joakim Zhang
Cc: peppe.cavallaro, alexandre.torgue, joabreu, davem, linux-imx,
netdev
On Thu, 3 Dec 2020 12:10:38 +0800 Joakim Zhang wrote:
> From: Fugang Duan <fugang.duan@nxp.com>
>
> Implement .set_intf_mode() callback for imx8dxl.
>
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Couple minor issues.
> @@ -86,7 +88,37 @@ imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
> {
> int ret = 0;
>
> - /* TBD: depends on imx8dxl scu interfaces to be upstreamed */
> + struct imx_sc_ipc *ipc_handle;
> + int val;
Looks like you're gonna have a empty line in the middle of variable
declarations?
Please remove it and order the variable lines longest to shortest.
> +
> + ret = imx_scu_get_handle(&ipc_handle);
> + if (ret)
> + return ret;
> +
> + switch (plat_dat->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + val = GPR_ENET_QOS_INTF_SEL_MII;
> + break;
> + case PHY_INTERFACE_MODE_RMII:
> + val = GPR_ENET_QOS_INTF_SEL_RMII;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + val = GPR_ENET_QOS_INTF_SEL_RGMII;
> + break;
> + default:
> + pr_debug("imx dwmac doesn't support %d interface\n",
> + plat_dat->interface);
> + return -EINVAL;
> + }
> +
> + ret = imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
> + IMX_SC_C_INTF_SEL, val >> 16);
> + ret |= imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
> + IMX_SC_C_CLK_GEN_EN, 0x1);
> return ret;
These calls may return different errors AFAICT.
You can't just errno values to gether the result will be meaningless.
please use the normal flow, and return the result of the second call
directly:
ret = func1();
if (ret)
return ret;
return func2();
Please also CC the maintainers of the Ethernet PHY subsystem on v2,
to make sure there is nothing wrong with the patch from their PoV.
Thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [PATCH] net: stmmac: implement .set_intf_mode() callback for imx8dxl
2020-12-05 19:58 ` Jakub Kicinski
@ 2020-12-07 8:15 ` Joakim Zhang
0 siblings, 0 replies; 3+ messages in thread
From: Joakim Zhang @ 2020-12-07 8:15 UTC (permalink / raw)
To: Jakub Kicinski
Cc: peppe.cavallaro@st.com, alexandre.torgue@st.com,
joabreu@synopsys.com, davem@davemloft.net, dl-linux-imx,
netdev@vger.kernel.org
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: 2020年12月6日 3:58
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: peppe.cavallaro@st.com; alexandre.torgue@st.com;
> joabreu@synopsys.com; davem@davemloft.net; dl-linux-imx
> <linux-imx@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH] net: stmmac: implement .set_intf_mode() callback for
> imx8dxl
>
> On Thu, 3 Dec 2020 12:10:38 +0800 Joakim Zhang wrote:
> > From: Fugang Duan <fugang.duan@nxp.com>
> >
> > Implement .set_intf_mode() callback for imx8dxl.
> >
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
>
> Couple minor issues.
>
> > @@ -86,7 +88,37 @@ imx8dxl_set_intf_mode(struct
> plat_stmmacenet_data
> > *plat_dat) {
> > int ret = 0;
> >
> > - /* TBD: depends on imx8dxl scu interfaces to be upstreamed */
> > + struct imx_sc_ipc *ipc_handle;
> > + int val;
>
> Looks like you're gonna have a empty line in the middle of variable
> declarations?
>
> Please remove it and order the variable lines longest to shortest.
>
> > +
> > + ret = imx_scu_get_handle(&ipc_handle);
> > + if (ret)
> > + return ret;
> > +
> > + switch (plat_dat->interface) {
> > + case PHY_INTERFACE_MODE_MII:
> > + val = GPR_ENET_QOS_INTF_SEL_MII;
> > + break;
> > + case PHY_INTERFACE_MODE_RMII:
> > + val = GPR_ENET_QOS_INTF_SEL_RMII;
> > + break;
> > + case PHY_INTERFACE_MODE_RGMII:
> > + case PHY_INTERFACE_MODE_RGMII_ID:
> > + case PHY_INTERFACE_MODE_RGMII_RXID:
> > + case PHY_INTERFACE_MODE_RGMII_TXID:
> > + val = GPR_ENET_QOS_INTF_SEL_RGMII;
> > + break;
> > + default:
> > + pr_debug("imx dwmac doesn't support %d interface\n",
> > + plat_dat->interface);
> > + return -EINVAL;
> > + }
> > +
> > + ret = imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
> > + IMX_SC_C_INTF_SEL, val >> 16);
> > + ret |= imx_sc_misc_set_control(ipc_handle, IMX_SC_R_ENET_1,
> > + IMX_SC_C_CLK_GEN_EN, 0x1);
> > return ret;
>
> These calls may return different errors AFAICT.
>
> You can't just errno values to gether the result will be meaningless.
>
> please use the normal flow, and return the result of the second call
> directly:
>
> ret = func1();
> if (ret)
> return ret;
>
> return func2();
>
> Please also CC the maintainers of the Ethernet PHY subsystem on v2, to make
> sure there is nothing wrong with the patch from their PoV.
Thanks Jakub for your kindly review, I will improve patch following your comments.
Best Regards,
Joakim Zhang
> Thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-07 8:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-03 4:10 [PATCH] net: stmmac: implement .set_intf_mode() callback for imx8dxl Joakim Zhang
2020-12-05 19:58 ` Jakub Kicinski
2020-12-07 8:15 ` Joakim Zhang
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).