[U-Boot] [PATCH] net: gmac_rockchip: Add support for pinctrl

Currently, selecting state simple is done by the device driver model, prior probing the driver. The problem is that it's done and called on the pinctrl node with "gmac" as the "periph" struct udevice *. So pinctrl-rk3288 is looking for an interrupt property that is not found, and then gmac_config is never called.
This commits toggles the pinctrl on the right node from the probe function of the driver.
Signed-off-by: Romain Perier romain.perier@collabora.com --- drivers/net/gmac_rockchip.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c index 5e2ca76302..8581026b4a 100644 --- a/drivers/net/gmac_rockchip.c +++ b/drivers/net/gmac_rockchip.c @@ -160,6 +160,10 @@ static int gmac_rockchip_probe(struct udevice *dev) struct clk clk; int ret;
+ ret = pinctrl_select_state(dev, "simple"); + if (ret) + return ret; + ret = clk_get_by_index(dev, 0, &clk); if (ret) return ret;

+Philipp
Hi Romain,
On 3 July 2017 at 01:13, Romain Perier romain.perier@collabora.com wrote:
Currently, selecting state simple is done by the device driver model, prior probing the driver. The problem is that it's done and called on the pinctrl node with "gmac" as the "periph" struct udevice *. So pinctrl-rk3288 is looking for an interrupt property that is not found, and then gmac_config is never called.
This commits toggles the pinctrl on the right node from the probe function of the driver.
Is it possible to fix this while still using driver-model automatic pinctrl?
Signed-off-by: Romain Perier romain.perier@collabora.com
drivers/net/gmac_rockchip.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c index 5e2ca76302..8581026b4a 100644 --- a/drivers/net/gmac_rockchip.c +++ b/drivers/net/gmac_rockchip.c @@ -160,6 +160,10 @@ static int gmac_rockchip_probe(struct udevice *dev) struct clk clk; int ret;
ret = pinctrl_select_state(dev, "simple");
if (ret)
return ret;
ret = clk_get_by_index(dev, 0, &clk); if (ret) return ret;
-- 2.11.0
Regards, Simon

Le 07/07/2017 à 05:58, Simon Glass a écrit :
+Philipp
Hi Romain,
On 3 July 2017 at 01:13, Romain Perier romain.perier@collabora.com wrote:
Currently, selecting state simple is done by the device driver model, prior probing the driver. The problem is that it's done and called on the pinctrl node with "gmac" as the "periph" struct udevice *. So pinctrl-rk3288 is looking for an interrupt property that is not found, and then gmac_config is never called.
This commits toggles the pinctrl on the right node from the probe function of the driver.
Is it possible to fix this while still using driver-model automatic pinctrl?
This is what I have tried to do, without success. The purpose of this patch is also to discuss about the possible solutions we have.
Regards, Romain
Signed-off-by: Romain Perier romain.perier@collabora.com
drivers/net/gmac_rockchip.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c index 5e2ca76302..8581026b4a 100644 --- a/drivers/net/gmac_rockchip.c +++ b/drivers/net/gmac_rockchip.c @@ -160,6 +160,10 @@ static int gmac_rockchip_probe(struct udevice *dev) struct clk clk; int ret;
ret = pinctrl_select_state(dev, "simple");
if (ret)
return ret;
ret = clk_get_by_index(dev, 0, &clk); if (ret) return ret;
-- 2.11.0
Regards, Simon

On 10 Jul 2017, at 08:43, Romain Perier romain.perier@collabora.com wrote:
Le 07/07/2017 à 05:58, Simon Glass a écrit :
+Philipp
Hi Romain,
On 3 July 2017 at 01:13, Romain Perier romain.perier@collabora.com wrote:
Currently, selecting state simple is done by the device driver model, prior probing the driver. The problem is that it's done and called on the pinctrl node with "gmac" as the "periph" struct udevice *. So pinctrl-rk3288 is looking for an interrupt property that is not found, and then gmac_config is never called.
This commits toggles the pinctrl on the right node from the probe function of the driver.
Is it possible to fix this while still using driver-model automatic pinctrl?
This is what I have tried to do, without success. The purpose of this patch is also to discuss about the possible solutions we have.
From looking at pinctrl_rk3288.c, I wonder if there’s a problem with pinctrl on the RK3288 outside of SPL: I see that the (full) set_state is also implemented outside of SPL (#ifndef CONFIG_SPL_BUILD) and not guarded by a CONFIG_IS_ENABLED(PINCTRL_FULL). So something might go down the wrong path.
Could start narrowing this down by enabling DEBUG in pinctrl_rk3288 to see whether this is even the simple pinctrl that gets called?
Regards, Romain
Signed-off-by: Romain Perier romain.perier@collabora.com
drivers/net/gmac_rockchip.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c index 5e2ca76302..8581026b4a 100644 --- a/drivers/net/gmac_rockchip.c +++ b/drivers/net/gmac_rockchip.c @@ -160,6 +160,10 @@ static int gmac_rockchip_probe(struct udevice *dev) struct clk clk; int ret;
ret = pinctrl_select_state(dev, "simple");
if (ret)
return ret;
ret = clk_get_by_index(dev, 0, &clk); if (ret) return ret;
-- 2.11.0
Regards, Simon

Hello,
Le 11/07/2017 à 15:43, Dr. Philipp Tomsich a écrit :
On 10 Jul 2017, at 08:43, Romain Perier romain.perier@collabora.com wrote:
Le 07/07/2017 à 05:58, Simon Glass a écrit :
+Philipp
Hi Romain,
On 3 July 2017 at 01:13, Romain Perier romain.perier@collabora.com wrote:
Currently, selecting state simple is done by the device driver model, prior probing the driver. The problem is that it's done and called on the pinctrl node with "gmac" as the "periph" struct udevice *. So pinctrl-rk3288 is looking for an interrupt property that is not found, and then gmac_config is never called.
This commits toggles the pinctrl on the right node from the probe function of the driver.
Is it possible to fix this while still using driver-model automatic pinctrl?
This is what I have tried to do, without success. The purpose of this patch is also to discuss about the possible solutions we have.
From looking at pinctrl_rk3288.c, I wonder if there’s a problem with pinctrl on the RK3288 outside of SPL: I see that the (full) set_state is also implemented outside of SPL (#ifndef CONFIG_SPL_BUILD) and not guarded by a CONFIG_IS_ENABLED(PINCTRL_FULL). So something might go down the wrong path.
Could start narrowing this down by enabling DEBUG in pinctrl_rk3288 to see whether this is even the simple pinctrl that gets called?
Yeah, the simple pinctrl is called at least for "hdmi" and the function for configuring hdmi is called internally.
Romain

On 07 Jul 2017, at 05:58, Simon Glass sjg@chromium.org wrote:
+Philipp
Hi Romain,
On 3 July 2017 at 01:13, Romain Perier romain.perier@collabora.com wrote:
Currently, selecting state simple is done by the device driver model, prior probing the driver. The problem is that it's done and called on the pinctrl node with "gmac" as the "periph" struct udevice *. So pinctrl-rk3288 is looking for an interrupt property that is not found, and then gmac_config is never called.
This commits toggles the pinctrl on the right node from the probe function of the driver.
Is it possible to fix this while still using driver-model automatic pinctrl?
I agree that we need to get a handle on the underlying issue and solve that instead of putting a band-aid on.
Signed-off-by: Romain Perier romain.perier@collabora.com
drivers/net/gmac_rockchip.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c index 5e2ca76302..8581026b4a 100644 --- a/drivers/net/gmac_rockchip.c +++ b/drivers/net/gmac_rockchip.c @@ -160,6 +160,10 @@ static int gmac_rockchip_probe(struct udevice *dev) struct clk clk; int ret;
ret = pinctrl_select_state(dev, "simple");
if (ret)
return ret;
ret = clk_get_by_index(dev, 0, &clk); if (ret) return ret;
-- 2.11.0
Regards, Simon
participants (3)
-
Dr. Philipp Tomsich
-
Romain Perier
-
Simon Glass