From: Evgeny Boger <boger@wirenboard.com> To: Maxime Ripard <maxime@cerno.tech> Cc: Chen-Yu Tsai <wens@csie.org>, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org> Subject: Re: [PATCH 1/2] net: allwinner: reset control support Date: Mon, 8 Mar 2021 16:51:14 +0300 [thread overview] Message-ID: <f4463df0-6669-fc20-ab8e-c82457e441fb@wirenboard.com> (raw) In-Reply-To: <20210308133617.xqbjv7jybxbbqa27@gilmour> Hi, thank you for your review! 3/8/21 4:36 PM, Maxime Ripard пишет: > Hi, > > On Sun, Mar 07, 2021 at 06:13:51AM +0300, Evgeny Boger wrote: >> R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP. >> However, on R40 the EMAC is gated by default. >> >> Signed-off-by: Evgeny Boger <boger@wirenboard.com> > On which device was it tested? It's custom-made Allwinner A40i device with two IP101GRI PHYs in MII mode. >> --- >> drivers/net/ethernet/allwinner/sun4i-emac.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c >> index 5ed80d9a6b9f..c0ae06dd922c 100644 >> --- a/drivers/net/ethernet/allwinner/sun4i-emac.c >> +++ b/drivers/net/ethernet/allwinner/sun4i-emac.c >> @@ -28,6 +28,7 @@ >> #include <linux/of_platform.h> >> #include <linux/platform_device.h> >> #include <linux/phy.h> >> +#include <linux/reset.h> >> #include <linux/soc/sunxi/sunxi_sram.h> >> >> #include "sun4i-emac.h" >> @@ -85,6 +86,7 @@ struct emac_board_info { >> unsigned int link; >> unsigned int speed; >> unsigned int duplex; >> + struct reset_control *reset; > You should align this with the rest of the other fields > >> >> phy_interface_t phy_interface; >> }; >> @@ -791,6 +793,7 @@ static int emac_probe(struct platform_device *pdev) >> struct net_device *ndev; >> int ret = 0; >> const char *mac_addr; >> + struct reset_control *reset; >> >> ndev = alloc_etherdev(sizeof(struct emac_board_info)); >> if (!ndev) { >> @@ -852,6 +855,19 @@ static int emac_probe(struct platform_device *pdev) >> goto out_release_sram; >> } >> >> + reset = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL); >> + if (IS_ERR(reset)) { >> + dev_err(&pdev->dev, "unable to request reset\n"); >> + ret = -ENODEV; >> + goto out_release_sram; >> + } > Judging from your commit log, it's not really optional for the R40. The > way we usually deal with this is to have a structure associated with a > new compatible and have a flag tell if that compatible requires a reset > line or not. > > The dt binding should also be amended to allow the reset property > got it >> + db->reset = reset; >> + ret = reset_control_deassert(db->reset); >> + if (ret) { >> + dev_err(&pdev->dev, "could not deassert EMAC reset\n"); >> + goto out_release_sram; >> + } >> + > The programming guidelines in the datasheet ask that the reset line must > be deasserted before the clock in enabled. right, found it at section 3.3.2.6, thanks > > Maxime
WARNING: multiple messages have this Message-ID (diff)
From: Evgeny Boger <boger@wirenboard.com> To: Maxime Ripard <maxime@cerno.tech> Cc: Chen-Yu Tsai <wens@csie.org>, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org> Subject: Re: [PATCH 1/2] net: allwinner: reset control support Date: Mon, 8 Mar 2021 16:51:14 +0300 [thread overview] Message-ID: <f4463df0-6669-fc20-ab8e-c82457e441fb@wirenboard.com> (raw) In-Reply-To: <20210308133617.xqbjv7jybxbbqa27@gilmour> Hi, thank you for your review! 3/8/21 4:36 PM, Maxime Ripard пишет: > Hi, > > On Sun, Mar 07, 2021 at 06:13:51AM +0300, Evgeny Boger wrote: >> R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP. >> However, on R40 the EMAC is gated by default. >> >> Signed-off-by: Evgeny Boger <boger@wirenboard.com> > On which device was it tested? It's custom-made Allwinner A40i device with two IP101GRI PHYs in MII mode. >> --- >> drivers/net/ethernet/allwinner/sun4i-emac.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c >> index 5ed80d9a6b9f..c0ae06dd922c 100644 >> --- a/drivers/net/ethernet/allwinner/sun4i-emac.c >> +++ b/drivers/net/ethernet/allwinner/sun4i-emac.c >> @@ -28,6 +28,7 @@ >> #include <linux/of_platform.h> >> #include <linux/platform_device.h> >> #include <linux/phy.h> >> +#include <linux/reset.h> >> #include <linux/soc/sunxi/sunxi_sram.h> >> >> #include "sun4i-emac.h" >> @@ -85,6 +86,7 @@ struct emac_board_info { >> unsigned int link; >> unsigned int speed; >> unsigned int duplex; >> + struct reset_control *reset; > You should align this with the rest of the other fields > >> >> phy_interface_t phy_interface; >> }; >> @@ -791,6 +793,7 @@ static int emac_probe(struct platform_device *pdev) >> struct net_device *ndev; >> int ret = 0; >> const char *mac_addr; >> + struct reset_control *reset; >> >> ndev = alloc_etherdev(sizeof(struct emac_board_info)); >> if (!ndev) { >> @@ -852,6 +855,19 @@ static int emac_probe(struct platform_device *pdev) >> goto out_release_sram; >> } >> >> + reset = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL); >> + if (IS_ERR(reset)) { >> + dev_err(&pdev->dev, "unable to request reset\n"); >> + ret = -ENODEV; >> + goto out_release_sram; >> + } > Judging from your commit log, it's not really optional for the R40. The > way we usually deal with this is to have a structure associated with a > new compatible and have a flag tell if that compatible requires a reset > line or not. > > The dt binding should also be amended to allow the reset property > got it >> + db->reset = reset; >> + ret = reset_control_deassert(db->reset); >> + if (ret) { >> + dev_err(&pdev->dev, "could not deassert EMAC reset\n"); >> + goto out_release_sram; >> + } >> + > The programming guidelines in the datasheet ask that the reset line must > be deasserted before the clock in enabled. right, found it at section 3.3.2.6, thanks > > Maxime _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-03-08 13:52 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-07 3:13 [PATCH 0/2] sun8i: r40: second ethernet support Evgeny Boger 2021-03-07 3:13 ` Evgeny Boger 2021-03-07 3:13 ` [PATCH 1/2] net: allwinner: reset control support Evgeny Boger 2021-03-07 3:13 ` Evgeny Boger 2021-03-08 13:36 ` Maxime Ripard 2021-03-08 13:36 ` Maxime Ripard 2021-03-08 13:51 ` Evgeny Boger [this message] 2021-03-08 13:51 ` Evgeny Boger 2021-03-07 3:13 ` [PATCH 2/2] dts: r40: add second ethernet support Evgeny Boger 2021-03-07 3:13 ` Evgeny Boger 2021-03-08 13:37 ` Maxime Ripard 2021-03-08 13:37 ` Maxime Ripard
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=f4463df0-6669-fc20-ab8e-c82457e441fb@wirenboard.com \ --to=boger@wirenboard.com \ --cc=devicetree@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maxime@cerno.tech \ --cc=robh+dt@kernel.org \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.