From: Saravana Kannan <saravanak@google.com> To: "Ondřej Jirman" <megous@megous.com>, "Saravana Kannan" <saravanak@google.com>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, linux-sunxi@lists.linux.dev, mripard@kernel.org, wens@csie.org, jernej.skrabec@gmail.com, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rafael J. Wysocki" <rafael@kernel.org>, LKML <linux-kernel@vger.kernel.org>, andre.przywara@arm.com Subject: Re: fw_devlink=on and sunxi HDMI Date: Wed, 19 May 2021 15:15:49 -0700 [thread overview] Message-ID: <CAGETcx-dS56NK3zuiUheB8ggavw-0O5G81N6Ea0LnoNxWm=OtA@mail.gmail.com> (raw) In-Reply-To: <20210519220002.w6e3jf6m5t45vbob@core> On Wed, May 19, 2021 at 3:00 PM Ondřej Jirman <megous@megous.com> wrote: > > Hello Saravana, > > On Wed, May 19, 2021 at 02:29:48PM -0700, Saravana Kannan wrote: > > > > Nothing in sun8i_hdmi_phy_probe() depends on anything from > > sun8i_dw_hdmi.c other than getting a struct device pointer to use with > > dev_err and some devm_* APIs. So it seems pretty straightforward to > > fix this so that you don't have one struct device trying to represent > > two distinct hardware blocks. What am I missing? > > > > Anyway, I took a swing at fixing this while preserving the ordering of > > the important bits. The changes are fairly trivial/straightforward and > > not meant to be final code, but can you test this out please? > > the patch seems to work, after fixing a few compilation issues. See > bellow. Thanks! Thanks for the quick test! Do you want to fix it up and get it upstreamed? I can do it, but I have other stuff I need to take care of. Also, I just wanted to prove the change I was suggesting wasn't complicated and fairly trivial. > I think the probe order is reversed, but HDMI works in my case, so > I guess that doesn't matter in the end. Or at least it didn't in the > short test I made. I don't think the "probe" order really changed. Firstly there was no real phy "probe" before. However, I'm guessing the order that really matters is the order between sun8i_hdmi_phy_init() and dw_hdmi_bind(). My patch still preserves that and that's why I think it works fine. sun8i_hdmi_phy_probe() was just doing a bunch of "gets" on resources so doing that a bit earlier seens pretty harmless. -Saravana > > kind regards, > o. > > > From 0eac644368711f52fffa4246aefc546591cef090 Mon Sep 17 00:00:00 2001 > From: Ondrej Jirman <megous@megous.com> > Date: Wed, 19 May 2021 23:44:45 +0200 > Subject: [PATCH] fix compilation issues > > --- > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > index c44ed22d8aef0..947b4231f6449 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > @@ -5,6 +5,7 @@ > > #include <linux/delay.h> > #include <linux/of_address.h> > +#include <linux/of_platform.h> > > #include "sun8i_dw_hdmi.h" > > @@ -748,7 +749,7 @@ int sun8i_hdmi_phy_probe(struct platform_device *pdev) > return ret; > } > > -void sun8i_hdmi_phy_remove(struct platform_device *pdev) > +int sun8i_hdmi_phy_remove(struct platform_device *pdev) > { > struct sun8i_hdmi_phy *phy = platform_get_drvdata(pdev); > > -- > 2.31.1 > > > [ 0.307254] platform 1c28000.serial: probe deferral - supplier 1c20800.pinctrl not ready > [ 0.307340] platform 1c28400.serial: probe deferral - supplier 1c20800.pinctrl not ready > [ 0.307379] platform 1c28c00.serial: probe deferral - supplier 1c20800.pinctrl not ready > [ 0.309235] fwnode_links_purge_suppliers: display-engine > [ 0.310176] platform 1c0c000.lcd-controller: probe deferral - wait for supplier mixer@200000 > [ 0.310228] platform 1c0d000.lcd-controller: probe deferral - wait for supplier mixer@200000 > [ 0.311902] platform 1ca0000.dsi: probe deferral - wait for supplier pmic@3a3 > [ 0.312287] platform 1ee0000.hdmi: probe deferral - wait for supplier pmic@3a3 > [ 0.313076] fwnode_links_purge_suppliers: hdmi-phy@1ef0000 <------ phy probed > [ 0.316766] platform 1c21800.lradc: probe deferral - wait for supplier pmic@3a3 > [ 0.317258] platform vibrator: probe deferral - wait for supplier pmic@3a3 > [ 0.318703] sun6i-rtc 1f00000.rtc: registered as rtc0 > [ 0.318748] sun6i-rtc 1f00000.rtc: setting system clock to 2021-05-19T21:40:59 UTC (1621460459) > [ 0.318917] sun6i-rtc 1f00000.rtc: RTC enabled > [ 0.318938] fwnode_links_purge_suppliers: rtc@1f00000 > [ 0.318960] fwnode_links_purge_consumers: reboot-mode@4 > [ 0.319185] i2c /dev entries driver > > .... > > [ 0.453301] fwnode_links_purge_consumers: port > [ 0.453323] fwnode_links_purge_consumers: endpoint > [ 0.453346] fwnode_links_purge_consumers: panel@0 > [ 0.491868] sun4i-drm display-engine: bound 1100000.mixer (ops 0xffffffc010a44230) > [ 0.501830] sun4i-drm display-engine: bound 1200000.mixer (ops 0xffffffc010a44230) > [ 0.502915] sun4i-drm display-engine: No panel or bridge found... RGB output disabled > [ 0.502956] sun4i-drm display-engine: bound 1c0c000.lcd-controller (ops 0xffffffc010a415a0) > [ 0.503466] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops 0xffffffc010a415a0) > [ 0.503545] sun4i-drm display-engine: bound 1ca0000.dsi (ops 0xffffffc010a431c8) > [ 0.505330] sun8i-dw-hdmi 1ee0000.hdmi: Detected HDMI TX controller v1.32a with HDCP (sun8i_dw_hdmi_phy) > [ 0.507566] sun8i-dw-hdmi 1ee0000.hdmi: registered DesignWare HDMI I2C bus driver > [ 0.510717] sun4i-drm display-engine: bound 1ee0000.hdmi (ops 0xffffffc010a43568) > [ 0.514674] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine on minor 0 > [ 0.514932] sun4i-drm display-engine: [drm] Cannot find any crtc or sizes > [ 0.515160] fwnode_links_purge_suppliers: hdmi@1ee0000 <------ hdmi probed > [ 0.515195] fwnode_links_purge_consumers: ports > [ 0.515217] fwnode_links_purge_consumers: port@0 > [ 0.515240] fwnode_links_purge_consumers: endpoint > [ 0.515264] fwnode_links_purge_consumers: port@1 > [ 0.515343] fwnode_links_purge_consumers: endpoint > [ 0.517089] input: 1c21800.lradc as /devices/platform/soc/1c21800.lradc/input/input1 > [ 0.517358] fwnode_links_purge_suppliers: lradc@1c21800 > [ 0.517384] fwnode_links_purge_consumers: button-200 > [ 0.517407] fwnode_links_purge_consumers: button-400 > [ 0.518414] input: gpio-vibrator as /devices/platform/vibrator/input/input2 > [ 0.518582] fwnode_links_purge_suppliers: vibrator > > > > > -Saravana
WARNING: multiple messages have this Message-ID (diff)
From: Saravana Kannan <saravanak@google.com> To: "Ondřej Jirman" <megous@megous.com>, "Saravana Kannan" <saravanak@google.com>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, linux-sunxi@lists.linux.dev, mripard@kernel.org, wens@csie.org, jernej.skrabec@gmail.com, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rafael J. Wysocki" <rafael@kernel.org>, LKML <linux-kernel@vger.kernel.org>, andre.przywara@arm.com Subject: Re: fw_devlink=on and sunxi HDMI Date: Wed, 19 May 2021 15:15:49 -0700 [thread overview] Message-ID: <CAGETcx-dS56NK3zuiUheB8ggavw-0O5G81N6Ea0LnoNxWm=OtA@mail.gmail.com> (raw) In-Reply-To: <20210519220002.w6e3jf6m5t45vbob@core> On Wed, May 19, 2021 at 3:00 PM Ondřej Jirman <megous@megous.com> wrote: > > Hello Saravana, > > On Wed, May 19, 2021 at 02:29:48PM -0700, Saravana Kannan wrote: > > > > Nothing in sun8i_hdmi_phy_probe() depends on anything from > > sun8i_dw_hdmi.c other than getting a struct device pointer to use with > > dev_err and some devm_* APIs. So it seems pretty straightforward to > > fix this so that you don't have one struct device trying to represent > > two distinct hardware blocks. What am I missing? > > > > Anyway, I took a swing at fixing this while preserving the ordering of > > the important bits. The changes are fairly trivial/straightforward and > > not meant to be final code, but can you test this out please? > > the patch seems to work, after fixing a few compilation issues. See > bellow. Thanks! Thanks for the quick test! Do you want to fix it up and get it upstreamed? I can do it, but I have other stuff I need to take care of. Also, I just wanted to prove the change I was suggesting wasn't complicated and fairly trivial. > I think the probe order is reversed, but HDMI works in my case, so > I guess that doesn't matter in the end. Or at least it didn't in the > short test I made. I don't think the "probe" order really changed. Firstly there was no real phy "probe" before. However, I'm guessing the order that really matters is the order between sun8i_hdmi_phy_init() and dw_hdmi_bind(). My patch still preserves that and that's why I think it works fine. sun8i_hdmi_phy_probe() was just doing a bunch of "gets" on resources so doing that a bit earlier seens pretty harmless. -Saravana > > kind regards, > o. > > > From 0eac644368711f52fffa4246aefc546591cef090 Mon Sep 17 00:00:00 2001 > From: Ondrej Jirman <megous@megous.com> > Date: Wed, 19 May 2021 23:44:45 +0200 > Subject: [PATCH] fix compilation issues > > --- > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > index c44ed22d8aef0..947b4231f6449 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > @@ -5,6 +5,7 @@ > > #include <linux/delay.h> > #include <linux/of_address.h> > +#include <linux/of_platform.h> > > #include "sun8i_dw_hdmi.h" > > @@ -748,7 +749,7 @@ int sun8i_hdmi_phy_probe(struct platform_device *pdev) > return ret; > } > > -void sun8i_hdmi_phy_remove(struct platform_device *pdev) > +int sun8i_hdmi_phy_remove(struct platform_device *pdev) > { > struct sun8i_hdmi_phy *phy = platform_get_drvdata(pdev); > > -- > 2.31.1 > > > [ 0.307254] platform 1c28000.serial: probe deferral - supplier 1c20800.pinctrl not ready > [ 0.307340] platform 1c28400.serial: probe deferral - supplier 1c20800.pinctrl not ready > [ 0.307379] platform 1c28c00.serial: probe deferral - supplier 1c20800.pinctrl not ready > [ 0.309235] fwnode_links_purge_suppliers: display-engine > [ 0.310176] platform 1c0c000.lcd-controller: probe deferral - wait for supplier mixer@200000 > [ 0.310228] platform 1c0d000.lcd-controller: probe deferral - wait for supplier mixer@200000 > [ 0.311902] platform 1ca0000.dsi: probe deferral - wait for supplier pmic@3a3 > [ 0.312287] platform 1ee0000.hdmi: probe deferral - wait for supplier pmic@3a3 > [ 0.313076] fwnode_links_purge_suppliers: hdmi-phy@1ef0000 <------ phy probed > [ 0.316766] platform 1c21800.lradc: probe deferral - wait for supplier pmic@3a3 > [ 0.317258] platform vibrator: probe deferral - wait for supplier pmic@3a3 > [ 0.318703] sun6i-rtc 1f00000.rtc: registered as rtc0 > [ 0.318748] sun6i-rtc 1f00000.rtc: setting system clock to 2021-05-19T21:40:59 UTC (1621460459) > [ 0.318917] sun6i-rtc 1f00000.rtc: RTC enabled > [ 0.318938] fwnode_links_purge_suppliers: rtc@1f00000 > [ 0.318960] fwnode_links_purge_consumers: reboot-mode@4 > [ 0.319185] i2c /dev entries driver > > .... > > [ 0.453301] fwnode_links_purge_consumers: port > [ 0.453323] fwnode_links_purge_consumers: endpoint > [ 0.453346] fwnode_links_purge_consumers: panel@0 > [ 0.491868] sun4i-drm display-engine: bound 1100000.mixer (ops 0xffffffc010a44230) > [ 0.501830] sun4i-drm display-engine: bound 1200000.mixer (ops 0xffffffc010a44230) > [ 0.502915] sun4i-drm display-engine: No panel or bridge found... RGB output disabled > [ 0.502956] sun4i-drm display-engine: bound 1c0c000.lcd-controller (ops 0xffffffc010a415a0) > [ 0.503466] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops 0xffffffc010a415a0) > [ 0.503545] sun4i-drm display-engine: bound 1ca0000.dsi (ops 0xffffffc010a431c8) > [ 0.505330] sun8i-dw-hdmi 1ee0000.hdmi: Detected HDMI TX controller v1.32a with HDCP (sun8i_dw_hdmi_phy) > [ 0.507566] sun8i-dw-hdmi 1ee0000.hdmi: registered DesignWare HDMI I2C bus driver > [ 0.510717] sun4i-drm display-engine: bound 1ee0000.hdmi (ops 0xffffffc010a43568) > [ 0.514674] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine on minor 0 > [ 0.514932] sun4i-drm display-engine: [drm] Cannot find any crtc or sizes > [ 0.515160] fwnode_links_purge_suppliers: hdmi@1ee0000 <------ hdmi probed > [ 0.515195] fwnode_links_purge_consumers: ports > [ 0.515217] fwnode_links_purge_consumers: port@0 > [ 0.515240] fwnode_links_purge_consumers: endpoint > [ 0.515264] fwnode_links_purge_consumers: port@1 > [ 0.515343] fwnode_links_purge_consumers: endpoint > [ 0.517089] input: 1c21800.lradc as /devices/platform/soc/1c21800.lradc/input/input1 > [ 0.517358] fwnode_links_purge_suppliers: lradc@1c21800 > [ 0.517384] fwnode_links_purge_consumers: button-200 > [ 0.517407] fwnode_links_purge_consumers: button-400 > [ 0.518414] input: gpio-vibrator as /devices/platform/vibrator/input/input2 > [ 0.518582] fwnode_links_purge_suppliers: vibrator > > > > > -Saravana _______________________________________________ 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-05-19 22:16 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-16 17:05 fw_devlink=on and sunxi HDMI Ondřej Jirman 2021-05-16 17:05 ` Ondřej Jirman 2021-05-17 6:32 ` Saravana Kannan 2021-05-17 6:32 ` Saravana Kannan 2021-05-17 8:29 ` Ondřej Jirman 2021-05-17 8:29 ` Ondřej Jirman 2021-05-18 14:04 ` Maxime Ripard 2021-05-18 14:04 ` Maxime Ripard 2021-05-19 21:29 ` Saravana Kannan 2021-05-19 21:29 ` Saravana Kannan 2021-05-19 22:00 ` Ondřej Jirman 2021-05-19 22:00 ` Ondřej Jirman 2021-05-19 22:15 ` Saravana Kannan [this message] 2021-05-19 22:15 ` Saravana Kannan
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='CAGETcx-dS56NK3zuiUheB8ggavw-0O5G81N6Ea0LnoNxWm=OtA@mail.gmail.com' \ --to=saravanak@google.com \ --cc=andre.przywara@arm.com \ --cc=gregkh@linuxfoundation.org \ --cc=jernej.skrabec@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sunxi@lists.linux.dev \ --cc=megous@megous.com \ --cc=mripard@kernel.org \ --cc=rafael@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.