Re: [PATCH v1] drm/mipi-dsi: Set the fwnode for mipi_dsi_device
On Thu, May 4, 2023 at 12:51 AM Maxime Ripard wrote:
>
> Hi Saravana,
>
> On Wed, May 03, 2023 at 09:40:05PM -0700, Saravana Kannan wrote:
> > On Fri, Mar 17, 2023 at 3:36 PM Saravana Kannan
> > wrote:
> > >
> > > On Sun, Mar 12, 2023 at 7:45 AM Martin Kepplinger
> > > wrote:
> > > >
> > > > Am Donnerstag, dem 09.03.2023 um 22:39 -0800 schrieb Saravana Kannan:
> > > > > After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
> > > > > detection more robust"), fw_devlink prints an error when consumer
> > > > > devices don't have their fwnode set. This used to be ignored
> > > > > silently.
> > > > >
> > > > > Set the fwnode mipi_dsi_device so fw_devlink can find them and
> > > > > properly
> > > > > track their dependencies.
> > > > >
> > > > > This fixes errors like this:
> > > > > [0.334054] nwl-dsi 30a0.mipi-dsi: Failed to create device
> > > > > link with regulator-lcd-1v8
> > > > > [0.346964] nwl-dsi 30a0.mipi-dsi: Failed to create device
> > > > > link with backlight-dsi
> > > > >
> > > > > Reported-by: Martin Kepplinger
> > > >
> > > > Reported-and-tested-by: Martin Kepplinger
> > >
> > > Maintainers,
> > >
> > > Nudge nudge. Will this be picked up for 6.3-rcX?
> >
> > Greg,
> >
> > Can you pick this up please? It's a fix that hasn't been picked up for
> > a few months.
> >
> > Here's the link to the actual patch for your convenience:
> > https://lore.kernel.org/lkml/20230310063910.2474472-1-sarava...@google.com/#t
>
> Sorry, I'm not quite sure what happened. I've applied it to drm-misc-fixes
No worries. Thanks Maxime!
-Saravana
Re: [PATCH v1] drm/mipi-dsi: Set the fwnode for mipi_dsi_device
On Fri, Mar 17, 2023 at 3:36 PM Saravana Kannan wrote:
>
> On Sun, Mar 12, 2023 at 7:45 AM Martin Kepplinger
> wrote:
> >
> > Am Donnerstag, dem 09.03.2023 um 22:39 -0800 schrieb Saravana Kannan:
> > > After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
> > > detection more robust"), fw_devlink prints an error when consumer
> > > devices don't have their fwnode set. This used to be ignored
> > > silently.
> > >
> > > Set the fwnode mipi_dsi_device so fw_devlink can find them and
> > > properly
> > > track their dependencies.
> > >
> > > This fixes errors like this:
> > > [0.334054] nwl-dsi 30a0.mipi-dsi: Failed to create device
> > > link with regulator-lcd-1v8
> > > [0.346964] nwl-dsi 30a0.mipi-dsi: Failed to create device
> > > link with backlight-dsi
> > >
> > > Reported-by: Martin Kepplinger
> >
> > Reported-and-tested-by: Martin Kepplinger
>
> Maintainers,
>
> Nudge nudge. Will this be picked up for 6.3-rcX?
Greg,
Can you pick this up please? It's a fix that hasn't been picked up for
a few months.
Here's the link to the actual patch for your convenience:
https://lore.kernel.org/lkml/20230310063910.2474472-1-sarava...@google.com/#t
-Saravana
>
> -Saravana
>
> >
> > thanks,
> > martin
> >
> > > Link:
> > > https://lore.kernel.org/lkml/2a8e407f4f18c9350f8629a2b5fa18673355b2ae.ca...@puri.sm/
> > > Fixes: 068a00233969 ("drm: Add MIPI DSI bus support")
> > > Signed-off-by: Saravana Kannan
> > > ---
> > > drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c
> > > b/drivers/gpu/drm/drm_mipi_dsi.c
> > > index b41aaf2bb9f1..7923cc21b78e 100644
> > > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > > @@ -221,7 +221,7 @@ mipi_dsi_device_register_full(struct
> > > mipi_dsi_host *host,
> > > return dsi;
> > > }
> > >
> > > - dsi->dev.of_node = info->node;
> > > + device_set_node(>dev, of_fwnode_handle(info->node));
> > > dsi->channel = info->channel;
> > > strlcpy(dsi->name, info->type, sizeof(dsi->name));
> > >
> >
> >
> > --
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to kernel-team+unsubscr...@android.com.
> >
Re: [PATCH v1] drm/mipi-dsi: Set the fwnode for mipi_dsi_device
On Sun, Mar 12, 2023 at 7:45 AM Martin Kepplinger
wrote:
>
> Am Donnerstag, dem 09.03.2023 um 22:39 -0800 schrieb Saravana Kannan:
> > After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
> > detection more robust"), fw_devlink prints an error when consumer
> > devices don't have their fwnode set. This used to be ignored
> > silently.
> >
> > Set the fwnode mipi_dsi_device so fw_devlink can find them and
> > properly
> > track their dependencies.
> >
> > This fixes errors like this:
> > [0.334054] nwl-dsi 30a0.mipi-dsi: Failed to create device
> > link with regulator-lcd-1v8
> > [0.346964] nwl-dsi 30a0.mipi-dsi: Failed to create device
> > link with backlight-dsi
> >
> > Reported-by: Martin Kepplinger
>
> Reported-and-tested-by: Martin Kepplinger
Maintainers,
Nudge nudge. Will this be picked up for 6.3-rcX?
-Saravana
>
> thanks,
> martin
>
> > Link:
> > https://lore.kernel.org/lkml/2a8e407f4f18c9350f8629a2b5fa18673355b2ae.ca...@puri.sm/
> > Fixes: 068a00233969 ("drm: Add MIPI DSI bus support")
> > Signed-off-by: Saravana Kannan
> > ---
> > drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c
> > b/drivers/gpu/drm/drm_mipi_dsi.c
> > index b41aaf2bb9f1..7923cc21b78e 100644
> > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > @@ -221,7 +221,7 @@ mipi_dsi_device_register_full(struct
> > mipi_dsi_host *host,
> > return dsi;
> > }
> >
> > - dsi->dev.of_node = info->node;
> > + device_set_node(>dev, of_fwnode_handle(info->node));
> > dsi->channel = info->channel;
> > strlcpy(dsi->name, info->type, sizeof(dsi->name));
> >
>
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-team+unsubscr...@android.com.
>
[PATCH v1] drm/mipi-dsi: Set the fwnode for mipi_dsi_device
After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
detection more robust"), fw_devlink prints an error when consumer
devices don't have their fwnode set. This used to be ignored silently.
Set the fwnode mipi_dsi_device so fw_devlink can find them and properly
track their dependencies.
This fixes errors like this:
[0.334054] nwl-dsi 30a0.mipi-dsi: Failed to create device link with
regulator-lcd-1v8
[0.346964] nwl-dsi 30a0.mipi-dsi: Failed to create device link with
backlight-dsi
Reported-by: Martin Kepplinger
Link:
https://lore.kernel.org/lkml/2a8e407f4f18c9350f8629a2b5fa18673355b2ae.ca...@puri.sm/
Fixes: 068a00233969 ("drm: Add MIPI DSI bus support")
Signed-off-by: Saravana Kannan
---
drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index b41aaf2bb9f1..7923cc21b78e 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -221,7 +221,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
return dsi;
}
- dsi->dev.of_node = info->node;
+ device_set_node(>dev, of_fwnode_handle(info->node));
dsi->channel = info->channel;
strlcpy(dsi->name, info->type, sizeof(dsi->name));
--
2.40.0.rc1.284.g88254d51c5-goog
Re: [PATCH v2 02/34] component: Introduce the aggregate bus_type
On Thu, Oct 7, 2021 at 1:11 PM Stephen Boyd wrote: > > Quoting Stephen Boyd (2021-10-07 11:40:07) > > Quoting Saravana Kannan (2021-10-06 20:07:11) > > > On Wed, Oct 6, 2021 at 12:38 PM Stephen Boyd wrote: > > > > diff --git a/drivers/base/component.c b/drivers/base/component.c > > > > index 0a41bbe14981..d99e99cabb99 100644 > > > > --- a/drivers/base/component.c > > > > +++ b/drivers/base/component.c > > [...] > > > > + continue; > > > > + > > > > + /* Matches put in component_del() */ > > > > + get_device(>dev); > > > > + c->link = device_link_add(>dev, c->dev, > > > > + DL_FLAG_STATELESS | > > > > DL_FLAG_PM_RUNTIME); > > > > > > Remove the STATELESS flag and you'll get a bunch of other stuff done for > > > free: > > > > I tried that and it didn't work for me. The aggregate device never > > probed and I was left with no display. Let me see if I can reproduce it > > with logging to provide more details. > > This patch fixes it (whitespace damaged sorry). Not sure why you have to trigger an explicit rescan, but instead of this patch below, you could also try setting this flag instead? DL_FLAG_AUTOPROBE_CONSUMER -Saravana > > 8< > diff --git a/drivers/base/component.c b/drivers/base/component.c > index 65042c9f8a42..43cac9ed70b7 100644 > --- a/drivers/base/component.c > +++ b/drivers/base/component.c > @@ -202,7 +202,7 @@ static int find_components(struct aggregate_device *adev) > /* Matches put in component_del() */ > get_device(>dev); > c->link = device_link_add(>dev, c->dev, > - DL_FLAG_STATELESS | > DL_FLAG_PM_RUNTIME); > + DL_FLAG_PM_RUNTIME); > c->adev = adev; > } > > @@ -749,7 +749,9 @@ static int __component_add(struct device *dev, > const struct component_ops *ops, > mutex_unlock(_mutex); > > /* Try to bind */ > - return bus_rescan_devices(_bus_type); > + bus_rescan_devices(_bus_type); > + > + return 0; > } > > /** > > > The important part is ignoring the return value of bus_rescan_devices(). > It's a cycle problem. The last component is probing and calling > component_add() in its probe function. The call to component_add() is > trying to probe the aggregate device now that all components are added. > But when it tries to probe the aggregate device it sees that a supplier, > which is this component calling compnent_add(), hasn't been probed yet, > so it returns -EPROBE_DEFER. That is passed up to the component and it > defers probe. > > I don't think the component device cares at all about the aggregate > device being able to probe or not. We should be able to ignore the > return value of bus_rescan_devices() in component_add(). I'll add a > comment to the code here so it's more obvious.
Re: [PATCH] drm/mipi: set fwnode when a mipi_dsi_device registers itself
Hi William, Thanks for catching this. On Fri, Jul 9, 2021 at 11:45 PM Will McVicker wrote: > > This is needed for fw_devlink to work properly with MIPI DSI devices. > Without setting the device's fwnode, the sync state framework isn't able > to properly track device links between the MIPI DSI device and its > suppliers which may result in its supplier probing before the mipi > device. I think it'd be more accurate if the commit text is something like: drm/mipi: set fwnode when a mipi_dsi_device is registered This allows the fw_devlink feature to work across mipi_dsi bus devices too. This feature avoid unnecessary probe deferrals of mipi_dsi devices, defers consumers of mipi_dsi devices till the mipi_dsi devices probe, and allows mipi_dsi drivers to implement sync_state() callbacks. Reviewed-by: Saravana Kannan Thanks, Saravana > > Suggested-by: Saravana Kannan > Signed-off-by: Will McVicker > --- > drivers/gpu/drm/drm_mipi_dsi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 5dd475e82995..469d56cf2a50 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -222,6 +222,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host, > } > > dsi->dev.of_node = info->node; > + dsi->dev.fwnode = of_fwnode_handle(info->node); > dsi->channel = info->channel; > strlcpy(dsi->name, info->type, sizeof(dsi->name)); > > -- > 2.32.0.93.g670b81a890-goog >
Re: [PATCH AUTOSEL 4.19 05/12] drm/sun4i: dw-hdmi: Make HDMI PHY into a platform device
On Tue, Jun 15, 2021 at 8:50 AM Sasha Levin wrote:
>
> From: Saravana Kannan
>
> [ Upstream commit 9bf3797796f570b34438235a6a537df85832bdad ]
>
> On sunxi boards that use HDMI output, HDMI device probe keeps being
> avoided indefinitely with these repeated messages in dmesg:
>
> platform 1ee.hdmi: probe deferral - supplier 1ef.hdmi-phy
> not ready
>
> There's a fwnode_link being created with fw_devlink=on between hdmi
> and hdmi-phy nodes, because both nodes have 'compatible' property set.
>
> Fw_devlink code assumes that nodes that have compatible property
> set will also have a device associated with them by some driver
> eventually. This is not the case with the current sun8i-hdmi
> driver.
Not needed. fw_devlink isn't present in 4.19.
-Saravana
>
> This commit makes sun8i-hdmi-phy into a proper platform device
> and fixes the display pipeline probe on sunxi boards that use HDMI.
>
> More context: https://lkml.org/lkml/2021/5/16/203
>
> Signed-off-by: Saravana Kannan
> Signed-off-by: Ondrej Jirman
> Tested-by: Andre Przywara
> Signed-off-by: Maxime Ripard
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210607085836.2827429-1-meg...@megous.com
> Signed-off-by: Sasha Levin
> ---
> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 31 ---
> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 5 ++--
> drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 41 ++
> 3 files changed, 66 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
> index 5073622cbb56..ab048f9412e7 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
> @@ -144,7 +144,7 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct
> device *master,
> goto err_disable_clk_tmds;
> }
>
> - ret = sun8i_hdmi_phy_probe(hdmi, phy_node);
> + ret = sun8i_hdmi_phy_get(hdmi, phy_node);
> of_node_put(phy_node);
> if (ret) {
> dev_err(dev, "Couldn't get the HDMI PHY\n");
> @@ -179,7 +179,6 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct
> device *master,
>
> cleanup_encoder:
> drm_encoder_cleanup(encoder);
> - sun8i_hdmi_phy_remove(hdmi);
> err_disable_clk_tmds:
> clk_disable_unprepare(hdmi->clk_tmds);
> err_assert_ctrl_reset:
> @@ -194,7 +193,6 @@ static void sun8i_dw_hdmi_unbind(struct device *dev,
> struct device *master,
> struct sun8i_dw_hdmi *hdmi = dev_get_drvdata(dev);
>
> dw_hdmi_unbind(hdmi->hdmi);
> - sun8i_hdmi_phy_remove(hdmi);
> clk_disable_unprepare(hdmi->clk_tmds);
> reset_control_assert(hdmi->rst_ctrl);
> }
> @@ -230,7 +228,32 @@ static struct platform_driver sun8i_dw_hdmi_pltfm_driver
> = {
> .of_match_table = sun8i_dw_hdmi_dt_ids,
> },
> };
> -module_platform_driver(sun8i_dw_hdmi_pltfm_driver);
> +
> +static int __init sun8i_dw_hdmi_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(_dw_hdmi_pltfm_driver);
> + if (ret)
> + return ret;
> +
> + ret = platform_driver_register(_hdmi_phy_driver);
> + if (ret) {
> + platform_driver_unregister(_dw_hdmi_pltfm_driver);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static void __exit sun8i_dw_hdmi_exit(void)
> +{
> + platform_driver_unregister(_dw_hdmi_pltfm_driver);
> + platform_driver_unregister(_hdmi_phy_driver);
> +}
> +
> +module_init(sun8i_dw_hdmi_init);
> +module_exit(sun8i_dw_hdmi_exit);
>
> MODULE_AUTHOR("Jernej Skrabec ");
> MODULE_DESCRIPTION("Allwinner DW HDMI bridge");
> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> index aadbe0a10b0c..41355bf3aca8 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> @@ -179,14 +179,15 @@ struct sun8i_dw_hdmi {
> struct reset_control*rst_ctrl;
> };
>
> +extern struct platform_driver sun8i_hdmi_phy_driver;
> +
> static inline struct sun8i_dw_hdmi *
> encoder_to_sun8i_dw_hdmi(struct drm_encoder *encoder)
> {
> return container_of(encoder, struct sun8i_dw_hdmi, encoder);
> }
>
> -int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, struct device_node
> *node);
> -void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi);
> +int sun8i_hdmi_phy_get(struct sun8i_dw_hdmi *hdmi, struct device_node *node);
>
> void sun8i_hdmi_phy_init(struct sun8i_
Re: [PATCH v2] drm/sun4i: dw-hdmi: Make HDMI PHY into a platform device
On Mon, Jun 7, 2021 at 4:41 AM Maxime Ripard wrote: > > On Mon, Jun 07, 2021 at 10:58:36AM +0200, Ondrej Jirman wrote: > > From: Saravana Kannan > > > > On sunxi boards that use HDMI output, HDMI device probe keeps being > > avoided indefinitely with these repeated messages in dmesg: > > > > platform 1ee.hdmi: probe deferral - supplier 1ef.hdmi-phy > > not ready > > > > There's a fwnode_link being created with fw_devlink=on between hdmi > > and hdmi-phy nodes, because both nodes have 'compatible' property set. > > > > Fw_devlink code assumes that nodes that have compatible property > > set will also have a device associated with them by some driver > > eventually. This is not the case with the current sun8i-hdmi > > driver. > > > > This commit makes sun8i-hdmi-phy into a proper platform device > > and fixes the display pipeline probe on sunxi boards that use HDMI. > > > > More context: https://lkml.org/lkml/2021/5/16/203 > > > > Signed-off-by: Saravana Kannan > > Signed-off-by: Ondrej Jirman > > Applied, thanks > Maxime Thanks everyone! And thanks for following up on this Ondrej! -Saravana
Re: [PATCH 3/7] component: Introduce struct aggregate_device
On Wed, May 19, 2021 at 5:25 PM Stephen Boyd wrote:
>
> Replace 'struct master' with 'struct aggregate_device' and then rename
> 'master' to 'adev' everywhere in the code. While we're here, put a
> struct device inside the aggregate device so that we can register it
> with a bus_type in the next patch.
>
> The diff is large but that's because this is mostly a rename, where
> sometimes 'master' is replaced with 'adev' and other times it is
> replaced with 'parent' to indicate that the struct device that was being
> used is actually the parent of the aggregate device and driver.
>
> Cc: Daniel Vetter
> Cc: "Rafael J. Wysocki"
> Cc: Rob Clark
> Cc: Russell King
> Cc: Saravana Kannan
> Signed-off-by: Stephen Boyd
> ---
> drivers/base/component.c | 249 --
> include/linux/component.h | 2 +-
> 2 files changed, 134 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 5e79299f6c3f..55e30e0b0952 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -9,6 +9,7 @@
> */
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -58,18 +59,21 @@ struct component_match {
> struct component_match_array *compare;
> };
>
> -struct master {
> +struct aggregate_device {
> struct list_head node;
> bool bound;
>
> const struct component_master_ops *ops;
> struct device *parent;
> + struct device dev;
> struct component_match *match;
> +
> + int id;
> };
>
> struct component {
> struct list_head node;
> - struct master *master;
> + struct aggregate_device *adev;
> bool bound;
>
> const struct component_ops *ops;
> @@ -79,7 +83,9 @@ struct component {
>
> static DEFINE_MUTEX(component_mutex);
> static LIST_HEAD(component_list);
> -static LIST_HEAD(masters);
> +static LIST_HEAD(aggregate_devices);
> +
> +static DEFINE_IDA(aggregate_ida);
>
> #ifdef CONFIG_DEBUG_FS
>
> @@ -87,12 +93,12 @@ static struct dentry *component_debugfs_dir;
>
> static int component_devices_show(struct seq_file *s, void *data)
> {
> - struct master *m = s->private;
> + struct aggregate_device *m = s->private;
> struct component_match *match = m->match;
> size_t i;
>
> mutex_lock(_mutex);
> - seq_printf(s, "%-40s %20s\n", "master name", "status");
> + seq_printf(s, "%-40s %20s\n", "aggregate_device name", "status");
> seq_puts(s,
> "-\n");
> seq_printf(s, "%-40s %20s\n\n",
>dev_name(m->parent), m->bound ? "bound" : "not bound");
> @@ -122,46 +128,46 @@ static int __init component_debug_init(void)
>
> core_initcall(component_debug_init);
>
> -static void component_master_debugfs_add(struct master *m)
> +static void component_master_debugfs_add(struct aggregate_device *m)
> {
> debugfs_create_file(dev_name(m->parent), 0444, component_debugfs_dir,
> m,
> _devices_fops);
> }
>
> -static void component_master_debugfs_del(struct master *m)
> +static void component_master_debugfs_del(struct aggregate_device *m)
> {
> debugfs_remove(debugfs_lookup(dev_name(m->parent),
> component_debugfs_dir));
> }
>
> #else
>
> -static void component_master_debugfs_add(struct master *m)
> +static void component_master_debugfs_add(struct aggregate_device *m)
> { }
>
> -static void component_master_debugfs_del(struct master *m)
> +static void component_master_debugfs_del(struct aggregate_device *m)
> { }
>
> #endif
>
> -static struct master *__master_find(struct device *parent,
> +static struct aggregate_device *__aggregate_find(struct device *parent,
> const struct component_master_ops *ops)
> {
> - struct master *m;
> + struct aggregate_device *m;
>
> - list_for_each_entry(m, , node)
> + list_for_each_entry(m, _devices, node)
> if (m->parent == parent && (!ops || m->ops == ops))
> return m;
>
> return NULL;
> }
>
> -static struct component *find_component(struct master *master,
> +static struct component *find_component(struct aggregate_device *adev,
> struct component_match_array *mc)
> {
> struct component *c;
>
> list_for_each_entry(c, _list, node) {
> - if
Re: [PATCH 0/7] component: Make into an aggregate bus
On Wed, May 19, 2021 at 6:41 PM Stephen Boyd wrote: > > Quoting Saravana Kannan (2021-05-19 18:27:50) > > On Wed, May 19, 2021 at 5:25 PM Stephen Boyd wrote: > > > > > > This series is from discussion we had on reordering the device lists for > > > drm shutdown paths[1]. I've introduced an 'aggregate' bus that we put > > > the aggregate device onto and then we probe the device once all the > > > components are probed and call component_add(). The probe/remove hooks > > > are where the bind/unbind calls go, and then a shutdown hook is added > > > that can be used to shutdown the drm display pipeline at the right time. > > > > > > This works for me on my sc7180 board, but I'm currently struggling with > > > the last patch where we migrate the msm driver. It runs into a runtime > > > PM problem where the parent device isn't runtime PM enabled yet. I'm > > > still trying to figure out a clean solution there. Moving runtime PM > > > around breaks boot and I think that's because the power domain is off. > > > > > > Cc: Daniel Vetter > > > Cc: "Rafael J. Wysocki" > > > Cc: Rob Clark > > > Cc: Russell King > > > Cc: Saravana Kannan > > > > > > [1] https://lore.kernel.org/r/20210508074118.1621729-1-swb...@chromium.org > > > > > > > I skimmed through the series and in general the idea is good, but I'm > > not sure why each component user needs to be converted/"modern" before > > it can make use of the benefits of this series. Why not just have > > wrapper functions around the component ops that the new aggregate bus > > driver can just call? That'll give all the existing component users > > the new ability to use the new ops without having to have two > > versions. > > The existing users can only have one or the other. Either use the ops > structure or use the struct aggregate_driver. What benefits of this > series are they not gaining? As I mentioned earlier, if we add device links between the aggregate device (consumer) and all the component devices (suppliers), it'll take care of a lot of the ordering issues (probe, suspend, runtime PM) and dependency issues (unbind the master device if a component driver unbinds). It'll allow us to delete a lot of the code in the component framework too. I can send the patch for the device links once your series settles. So having two implementations comes in the way of a clean up and code improvement because we'll have to keep a lot of the component code for the purpose of the "legacy" ops. > > That'll also allow us to do other improvements (I have some > > in mind) that'll apply to all the component users instead of only the > > converted ones. > > What do you have in mind? I didn't want to convert drivers over to the > new way of doing things without making them consciously change their > code. What ordering/behavior would you be changing with the new ops? If the new shutdown ops isn't used, it really shouldn't change anything. Put another way, if we ignore your msm driver changes, we should be able to switch to having a real device for the "master" without making any functional change. If you are causing any functional change with the new ops, maybe you can key it off a flag that needs to be set? That way, we'll have one API/ops but still be backward compatible if you are worried about breaking existing users? > Otherwise I worry it will break things in random, subtle ways. The > last patch, as I mentioned above in the cover, causes warnings because > the display driver is enabling runtime PM in an odd spot as part of the > bind callback of the aggregate/master. That should move out of there and > into the msm_pdev driver that registers the aggregate from what I can > tell. Can you give more context? I think if you create device links with RPM_ACTIVE and PM_RUNTIME flags, it should ensure runtime PM correctness. -Saravana
Re: [PATCHv2 1/5] rtc: m41t80: add support for fixed clock
On Wed, Apr 28, 2021 at 3:29 PM Sebastian Reichel
wrote:
>
> Congatec's QMX6 system on module (SoM) uses a m41t62 as RTC. The
> modules SQW clock output defaults to 32768 Hz. This behaviour is
> used to provide the i.MX6 CKIL clock. Once the RTC driver is probed,
> the clock is disabled and all i.MX6 functionality depending on
> the 32 KHz clock has undefined behaviour. For example when using
> the hardware watchdog the system will likely do arbitrary reboots.
>
> Referencing the m41t62 directly results in a deadlock. The kernel
> will see, that i.MX6 system clock needs the RTC clock and do probe
> deferral. But the i.MX6 I2C module never becomes usable without the
> i.MX6 CKIL clock and thus the RTC's clock will not be probed. So
> from the kernel's perspective this is a chicken-and-egg problem.
>
> Technically everything is fine by not touching anything, since
> the RTC clock correctly enables the clock on reset (i.e. on
> battery backup power loss) and also the bootloader enables it
> in case an something (e.g. an unpatched kernel) disabled this
> incorrectly.
>
> A workaround for this issue is describing the square wave pin
> as fixed-clock, which is registered early and basically how
> this pin is used on the i.MX6.
>
> Suggested-by: Saravana Kannan
> Signed-off-by: Sebastian Reichel
> ---
> Documentation/devicetree/bindings/rtc/rtc-m41t80.txt | 9 +
> drivers/rtc/rtc-m41t80.c | 12
> 2 files changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
> b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
> index c746cb221210..cdd196b1e9bd 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
> @@ -21,10 +21,19 @@ Optional properties:
>clock name
> - wakeup-source: Enables wake up of host system on alarm
>
> +Optional child node:
> +- clock: Provide this if the square wave pin is used as boot-enabled fixed
> clock.
> +
> Example:
> rtc@68 {
> compatible = "st,m41t80";
> reg = <0x68>;
> interrupt-parent = <>;
> interrupts = <0x9 0x8>;
> +
> + clock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + };
> };
> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
> index 89128fc29ccc..b3ece42b6f90 100644
> --- a/drivers/rtc/rtc-m41t80.c
> +++ b/drivers/rtc/rtc-m41t80.c
> @@ -544,10 +544,22 @@ static struct clk *m41t80_sqw_register_clk(struct
> m41t80_data *m41t80)
> {
> struct i2c_client *client = m41t80->client;
> struct device_node *node = client->dev.of_node;
> + struct device_node *fixed_clock;
> struct clk *clk;
> struct clk_init_data init;
> int ret;
>
> + fixed_clock = of_get_child_by_name(node, "clock");
> + if (fixed_clock) {
> + /*
> +* skip registering square wave clock when a fixed
> +* clock has been registered. The fixed clock is
> +* registered automatically when being referenced.
> +*/
> + of_node_put(fixed_clock);
> + return 0;
> + }
> +
> /* First disable the clock */
> ret = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_MON);
> if (ret < 0)
Reviewed-by: Saravana Kannan
-Saravana
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Closed source userspace graphics drivers with an open source kernel component
Dave Airlie wrote: > This is more about initial development stages. We maintain kernel > API/ABI for all in-tree drivers, however before we put a driver into > mainline, we usually need to redo the crazy interfaces that vendors > have come up with. Like 32/64 alignment, passing userspace addresses > into the kernel, passing phy addresses to userspace etc. If the > userspace binary is closed that process becomes next to impossible. My 2 cents: I think we should leave the onus of fixing the userspace to work with the sane kernel API with the entity trying to get their drivers into the kernel. I think it's a better approach (as in, more likelihood of getting device support) than saying, we will only allow fully open sourced kernel drivers. -Saravana
