[PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[Liu Ying]

Hi,

This series aims to check panel device dependency upon DRM device before
managing device link between them.  It fixes eariler patches in v6.7-rc1
which tried to manage the link.  Without this series, the link fails to
be added for dependent panel devices and hence relevant panel bridges
fail to be attached.  A real broken panel is "novatek,nt35510" defined
in arch/arm/boot/dts/st/ste-ux500-samsung-skomer.dts as reported by
Linus Walleij.

Patch 1 exports device_is_dependent() to modules as needed by patch 2.
Patch 2 checks device dependency before managing the device link.

Note that patch 2 is already in drm-misc/drm-misc-fixes and
drm-misc/for-linux-next-fixes.  Patch 1 needs to be reviewed and picked up.

v2:
* Introduce patch 1 to export device_is_dependent() to modules as needed by
  patch 2.

Liu Ying (2):
  driver core: Export device_is_dependent() to modules
  drm/bridge: panel: Check device dependency before managing device link

 drivers/base/core.c            |  1 +
 drivers/gpu/drm/bridge/panel.c | 27 ++++++++++++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)

-- 
2.37.1

[PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Liu Ying]

Export device_is_dependent() since the drm_kms_helper module is starting
to use it.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v2:
* Newly introduced as needed by patch 2.

 drivers/base/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 67ba592afc77..bfd2bf0364b7 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -328,6 +328,7 @@ int device_is_dependent(struct device *dev, void *target)
 	}
 	return ret;
 }
+EXPORT_SYMBOL_GPL(device_is_dependent);
 
 static void device_link_init_status(struct device_link *link,
 				    struct device *consumer,
-- 
2.37.1

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

Greg, Rafael,

On Mon, Nov 27, 2023 at 01:14:13PM +0800, Liu Ying wrote:
> Export device_is_dependent() since the drm_kms_helper module is starting
> to use it.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> v2:
> * Newly introduced as needed by patch 2.
> 
>  drivers/base/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 67ba592afc77..bfd2bf0364b7 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -328,6 +328,7 @@ int device_is_dependent(struct device *dev, void *target)
>  	}
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(device_is_dependent);

So, a committer just applied this to drm-misc-fixes without your
approval. Could you ack it? If you don't want to, we'll fix it.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Greg KH]

On Mon, Nov 27, 2023 at 05:38:13PM +0100, Maxime Ripard wrote:
> Greg, Rafael,
> 
> On Mon, Nov 27, 2023 at 01:14:13PM +0800, Liu Ying wrote:
> > Export device_is_dependent() since the drm_kms_helper module is starting
> > to use it.
> > 
> > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > ---
> > v2:
> > * Newly introduced as needed by patch 2.
> > 
> >  drivers/base/core.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 67ba592afc77..bfd2bf0364b7 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -328,6 +328,7 @@ int device_is_dependent(struct device *dev, void *target)
> >  	}
> >  	return ret;
> >  }
> > +EXPORT_SYMBOL_GPL(device_is_dependent);
> 
> So, a committer just applied this to drm-misc-fixes without your
> approval. Could you ack it? If you don't want to, we'll fix it.

Wait, why exactly is this needed?  Nothing outside of the driver core
should be needing this function, it shouldn't be public at all (I missed
that before.)

So please, revert it for now, let's figure out why DRM thinks this is
needed for it's devices, and yet no other bus/subsystem does.

thanks,

greg k-h

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Linus Walleij]

On Mon, Nov 27, 2023 at 7:20 PM Greg KH <gregkh@linuxfoundation.org> wrote:

[Maxime]
> > So, a committer just applied this to drm-misc-fixes without your
> > approval.

That was me, mea culpa.
I learned a lesson. Or two.

> Wait, why exactly is this needed?  Nothing outside of the driver core
> should be needing this function, it shouldn't be public at all (I missed
> that before.)
>
> So please, revert it for now, let's figure out why DRM thinks this is
> needed for it's devices, and yet no other bus/subsystem does.

I'm preparing a revert series back to the original patch causing
the issue so we can clear the original bug out of the way, take
a step back and fix this properly.

Yours,
Linus Walleij

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Bartosz Golaszewski]

On Mon, Nov 27, 2023 at 7:21 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Nov 27, 2023 at 05:38:13PM +0100, Maxime Ripard wrote:
> > Greg, Rafael,
> >
> > On Mon, Nov 27, 2023 at 01:14:13PM +0800, Liu Ying wrote:
> > > Export device_is_dependent() since the drm_kms_helper module is starting
> > > to use it.
> > >
> > > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > > ---
> > > v2:
> > > * Newly introduced as needed by patch 2.
> > >
> > >  drivers/base/core.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > index 67ba592afc77..bfd2bf0364b7 100644
> > > --- a/drivers/base/core.c
> > > +++ b/drivers/base/core.c
> > > @@ -328,6 +328,7 @@ int device_is_dependent(struct device *dev, void *target)
> > >     }
> > >     return ret;
> > >  }
> > > +EXPORT_SYMBOL_GPL(device_is_dependent);
> >
> > So, a committer just applied this to drm-misc-fixes without your
> > approval. Could you ack it? If you don't want to, we'll fix it.
>
> Wait, why exactly is this needed?  Nothing outside of the driver core
> should be needing this function, it shouldn't be public at all (I missed
> that before.)
>

Hi Greg!

Sorry for reviving this old thread but I stumbled upon it when looking
for information on whether anyone ever tried to export
device_is_dependent() before.

I have a use-case where I think I need to check if two devices are
linked with a device link. I assume you'll prove me wrong. :)

The reset-gpio driver is a reset control driver that mediates sharing
a reset GPIO (defined as a standardized reference fwnode property
"reset-gpios") among multiple users. reset-gpio auxiliary devices are
instantiated from reset core when it detects a consumer trying to get
a reset-control handle but there's no "resets" reference on the
consumer's fwnode, only "reset-gpios".

It makes sense for reset core to create a device link between the
auxiliary reset-gpio device (the supplier) and the reset consumer
because this link is not described in firmware - only the link between
the consumer AND the GPIO controller.

Now in order to make gpiolib-shared.c code (generic support for shared
GPIOs) happy, I need to handle the side effects of interacting with
reset-gpio which does a similar thing. To that end, I need to know if
given GPIO controller is a supplier of the consumer described in
firmware OR the auxiliary reset device which is only described with
software nodes.

The logical thing to do would be to use "device_is_dependent()" but
this thread makes me think that won't fly.

What should I do? What's the "correct" way of checking if two devices
are linked? I assume that fiddling with the supplier/consumer lists in
struct device is not it.

Thanks,
Bartosz

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Greg KH]

On Wed, Nov 26, 2025 at 02:13:03PM +0100, Bartosz Golaszewski wrote:
> On Mon, Nov 27, 2023 at 7:21 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Nov 27, 2023 at 05:38:13PM +0100, Maxime Ripard wrote:
> > > Greg, Rafael,
> > >
> > > On Mon, Nov 27, 2023 at 01:14:13PM +0800, Liu Ying wrote:
> > > > Export device_is_dependent() since the drm_kms_helper module is starting
> > > > to use it.
> > > >
> > > > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > > > ---
> > > > v2:
> > > > * Newly introduced as needed by patch 2.
> > > >
> > > >  drivers/base/core.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > > index 67ba592afc77..bfd2bf0364b7 100644
> > > > --- a/drivers/base/core.c
> > > > +++ b/drivers/base/core.c
> > > > @@ -328,6 +328,7 @@ int device_is_dependent(struct device *dev, void *target)
> > > >     }
> > > >     return ret;
> > > >  }
> > > > +EXPORT_SYMBOL_GPL(device_is_dependent);
> > >
> > > So, a committer just applied this to drm-misc-fixes without your
> > > approval. Could you ack it? If you don't want to, we'll fix it.
> >
> > Wait, why exactly is this needed?  Nothing outside of the driver core
> > should be needing this function, it shouldn't be public at all (I missed
> > that before.)
> >
> 
> Hi Greg!
> 
> Sorry for reviving this old thread but I stumbled upon it when looking
> for information on whether anyone ever tried to export
> device_is_dependent() before.
> 
> I have a use-case where I think I need to check if two devices are
> linked with a device link. I assume you'll prove me wrong. :)
> 
> The reset-gpio driver is a reset control driver that mediates sharing
> a reset GPIO (defined as a standardized reference fwnode property
> "reset-gpios") among multiple users. reset-gpio auxiliary devices are
> instantiated from reset core when it detects a consumer trying to get
> a reset-control handle but there's no "resets" reference on the
> consumer's fwnode, only "reset-gpios".
> 
> It makes sense for reset core to create a device link between the
> auxiliary reset-gpio device (the supplier) and the reset consumer
> because this link is not described in firmware - only the link between
> the consumer AND the GPIO controller.
> 
> Now in order to make gpiolib-shared.c code (generic support for shared
> GPIOs) happy, I need to handle the side effects of interacting with
> reset-gpio which does a similar thing. To that end, I need to know if
> given GPIO controller is a supplier of the consumer described in
> firmware OR the auxiliary reset device which is only described with
> software nodes.
> 
> The logical thing to do would be to use "device_is_dependent()" but
> this thread makes me think that won't fly.
> 
> What should I do? What's the "correct" way of checking if two devices
> are linked? I assume that fiddling with the supplier/consumer lists in
> struct device is not it.

fiddling with those lists is what device_is_dependent() does, but no,
you really don't want to be doing that either manually or by calling
this function.

Who is creating this "link"?  Can't that caller tell the gpio core this
relationship at the same time as you are wanting to keep track of it
too?

thanks,

greg k-h

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Bartosz Golaszewski]

On Thu, 27 Nov 2025 09:29:03 +0100, Greg KH <gregkh@linuxfoundation.org> said:
> On Wed, Nov 26, 2025 at 02:13:03PM +0100, Bartosz Golaszewski wrote:
>> The logical thing to do would be to use "device_is_dependent()" but
>> this thread makes me think that won't fly.
>>
>> What should I do? What's the "correct" way of checking if two devices
>> are linked? I assume that fiddling with the supplier/consumer lists in
>> struct device is not it.
>
> fiddling with those lists is what device_is_dependent() does, but no,
> you really don't want to be doing that either manually or by calling
> this function.
>
> Who is creating this "link"?  Can't that caller tell the gpio core this
> relationship at the same time as you are wanting to keep track of it
> too?
>

The link would be created in reset core.

Let's consider the following:

GPIO Consumer A ---> reset-gpio ---> |
                                     | GPIO Controller pin X
GPIO Consumer B -------------------> |

The GPIO core will scan the device tree and realize that A and B share the
same pin. The reset-gpio device is not described in firmware, it will be
created only when A requests a reset control. When it, on behalf of consumer A,
requests pin X, GPIO core can not associate the link between consumer A and
pin X with the link between reset-gpio and pin X because there's no such
reference in firmware nodes between consumer A and reset-gpio. To GPIO, these
are two separate references to the same pin. Only reset core knows about A
being the consumer of reset-gpio.

We could add a function in reset like:

  struct device *reset_gpio_to_reset_device(struct device *dev);

which would return the actual consumer of pin X for a device we identified as
reset-gpio (for instance: with device_is_compatible(dev, "reset-gpio")) but
that would be adding a symbol for a corner case and a single user and for
which the relevant information already exists and could easily be retrieved
from existing device links.

I hope that explains it better.

To answer your question: the caller can't tell GPIO about this relationship,
GPIO would have to ask reset about it but having a dedicated symbol for this
doesn't really sound like the best approach.

Bartosz

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Greg KH]

On Thu, Nov 27, 2025 at 05:19:19AM -0800, Bartosz Golaszewski wrote:
> On Thu, 27 Nov 2025 09:29:03 +0100, Greg KH <gregkh@linuxfoundation.org> said:
> > On Wed, Nov 26, 2025 at 02:13:03PM +0100, Bartosz Golaszewski wrote:
> >> The logical thing to do would be to use "device_is_dependent()" but
> >> this thread makes me think that won't fly.
> >>
> >> What should I do? What's the "correct" way of checking if two devices
> >> are linked? I assume that fiddling with the supplier/consumer lists in
> >> struct device is not it.
> >
> > fiddling with those lists is what device_is_dependent() does, but no,
> > you really don't want to be doing that either manually or by calling
> > this function.
> >
> > Who is creating this "link"?  Can't that caller tell the gpio core this
> > relationship at the same time as you are wanting to keep track of it
> > too?
> >
> 
> The link would be created in reset core.
> 
> Let's consider the following:
> 
> GPIO Consumer A ---> reset-gpio ---> |
>                                      | GPIO Controller pin X
> GPIO Consumer B -------------------> |
> 
> The GPIO core will scan the device tree and realize that A and B share the
> same pin. The reset-gpio device is not described in firmware, it will be
> created only when A requests a reset control. When it, on behalf of consumer A,
> requests pin X, GPIO core can not associate the link between consumer A and
> pin X with the link between reset-gpio and pin X because there's no such
> reference in firmware nodes between consumer A and reset-gpio. To GPIO, these
> are two separate references to the same pin. Only reset core knows about A
> being the consumer of reset-gpio.
> 
> We could add a function in reset like:
> 
>   struct device *reset_gpio_to_reset_device(struct device *dev);
> 
> which would return the actual consumer of pin X for a device we identified as
> reset-gpio (for instance: with device_is_compatible(dev, "reset-gpio")) but
> that would be adding a symbol for a corner case and a single user and for
> which the relevant information already exists and could easily be retrieved
> from existing device links.
> 
> I hope that explains it better.

Yes it does, thanks.

> To answer your question: the caller can't tell GPIO about this relationship,
> GPIO would have to ask reset about it but having a dedicated symbol for this
> doesn't really sound like the best approach.

Ah, ick, no it doesn't.  I really don't know what to suggest here,
sorry.

greg k-h

Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules

[Bartosz Golaszewski]

On Wed, Dec 3, 2025 at 6:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > I hope that explains it better.
>
> Yes it does, thanks.
>
> > To answer your question: the caller can't tell GPIO about this relationship,
> > GPIO would have to ask reset about it but having a dedicated symbol for this
> > doesn't really sound like the best approach.
>
> Ah, ick, no it doesn't.  I really don't know what to suggest here,
> sorry.
>

I found a viable workaround inside GPIO where I create another GPIO
shared proxy for the potential reset-gpio device. So that if there are
two users of the same "reset-gpios", we create three proxies in total:
one for user 1, one for user 2 and another one for the reset-gpio
device which may or may not be instantiated.

I think that the best approach would still be having access to
device_is_dependent(). I don't quite get why read-only inspecting
device links outside of PM or driver core should really be a bad
thing, but I can live without it, I guess.

Bart

[PATCH v2 2/2] drm/bridge: panel: Check device dependency before managing device link

[Liu Ying]

Some panel devices already depend on DRM device, like the panel in
arch/arm/boot/dts/st/ste-ux500-samsung-skomer.dts, because DRM device is
the ancestor of those panel devices.  device_link_add() would fail by
returning a NULL pointer for those panel devices because of the existing
dependency.  So, check the dependency by calling device_is_dependent()
before adding or deleting device link between panel device and DRM device
so that the link is managed only for independent panel devices.

Fixes: 887878014534 ("drm/bridge: panel: Fix device link for DRM_BRIDGE_ATTACH_NO_CONNECTOR")
Fixes: 199cf07ebd2b ("drm/bridge: panel: Add a device link between drm device and panel device")
Reported-by: Linus Walleij <linus.walleij@linaro.org>
Closes: https://lore.kernel.org/lkml/CACRpkdaGzXD6HbiX7mVUNJAJtMEPG00Pp6+nJ1P0JrfJ-ArMvQ@mail.gmail.com/T/
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
v2:
* No change.

 drivers/gpu/drm/bridge/panel.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index e48823a4f1ed..5e8980023407 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -23,6 +23,7 @@ struct panel_bridge {
 	struct drm_panel *panel;
 	struct device_link *link;
 	u32 connector_type;
+	bool is_independent;
 };
 
 static inline struct panel_bridge *
@@ -67,12 +68,17 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
 	struct drm_device *drm_dev = bridge->dev;
 	int ret;
 
-	panel_bridge->link = device_link_add(drm_dev->dev, panel->dev,
-					     DL_FLAG_STATELESS);
-	if (!panel_bridge->link) {
-		DRM_ERROR("Failed to add device link between %s and %s\n",
-			  dev_name(drm_dev->dev), dev_name(panel->dev));
-		return -EINVAL;
+	panel_bridge->is_independent = !device_is_dependent(drm_dev->dev,
+							    panel->dev);
+
+	if (panel_bridge->is_independent) {
+		panel_bridge->link = device_link_add(drm_dev->dev, panel->dev,
+						     DL_FLAG_STATELESS);
+		if (!panel_bridge->link) {
+			DRM_ERROR("Failed to add device link between %s and %s\n",
+				  dev_name(drm_dev->dev), dev_name(panel->dev));
+			return -EINVAL;
+		}
 	}
 
 	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
@@ -80,7 +86,8 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
 
 	if (!bridge->encoder) {
 		DRM_ERROR("Missing encoder\n");
-		device_link_del(panel_bridge->link);
+		if (panel_bridge->is_independent)
+			device_link_del(panel_bridge->link);
 		return -ENODEV;
 	}
 
@@ -92,7 +99,8 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
 				 panel_bridge->connector_type);
 	if (ret) {
 		DRM_ERROR("Failed to initialize connector\n");
-		device_link_del(panel_bridge->link);
+		if (panel_bridge->is_independent)
+			device_link_del(panel_bridge->link);
 		return ret;
 	}
 
@@ -115,7 +123,8 @@ static void panel_bridge_detach(struct drm_bridge *bridge)
 	struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
 	struct drm_connector *connector = &panel_bridge->connector;
 
-	device_link_del(panel_bridge->link);
+	if (panel_bridge->is_independent)
+		device_link_del(panel_bridge->link);
 
 	/*
 	 * Cleanup the connector if we know it was initialized.
-- 
2.37.1

Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[Linus Walleij]

On Mon, Nov 27, 2023 at 6:10 AM Liu Ying <victor.liu@nxp.com> wrote:

> This series aims to check panel device dependency upon DRM device before
> managing device link between them.  It fixes eariler patches in v6.7-rc1
> which tried to manage the link.  Without this series, the link fails to
> be added for dependent panel devices and hence relevant panel bridges
> fail to be attached.  A real broken panel is "novatek,nt35510" defined
> in arch/arm/boot/dts/st/ste-ux500-samsung-skomer.dts as reported by
> Linus Walleij.
>
> Patch 1 exports device_is_dependent() to modules as needed by patch 2.
> Patch 2 checks device dependency before managing the device link.
>
> Note that patch 2 is already in drm-misc/drm-misc-fixes and
> drm-misc/for-linux-next-fixes.  Patch 1 needs to be reviewed and picked up.
>
> v2:
> * Introduce patch 1 to export device_is_dependent() to modules as needed by
>   patch 2.
>
> Liu Ying (2):
>   driver core: Export device_is_dependent() to modules
>   drm/bridge: panel: Check device dependency before managing device link

I just applied patch 1 directly to the drm-misc-fixes so we don't have to
revert and then re-apply patches, because that is a bigger evil. (We can't
rebase these branches...)

Yours,
Linus Walleij

Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]

On Mon, Nov 27, 2023 at 05:03:53PM +0100, Linus Walleij wrote:
> On Mon, Nov 27, 2023 at 6:10 AM Liu Ying <victor.liu@nxp.com> wrote:
> 
> > This series aims to check panel device dependency upon DRM device before
> > managing device link between them.  It fixes eariler patches in v6.7-rc1
> > which tried to manage the link.  Without this series, the link fails to
> > be added for dependent panel devices and hence relevant panel bridges
> > fail to be attached.  A real broken panel is "novatek,nt35510" defined
> > in arch/arm/boot/dts/st/ste-ux500-samsung-skomer.dts as reported by
> > Linus Walleij.
> >
> > Patch 1 exports device_is_dependent() to modules as needed by patch 2.
> > Patch 2 checks device dependency before managing the device link.
> >
> > Note that patch 2 is already in drm-misc/drm-misc-fixes and
> > drm-misc/for-linux-next-fixes.  Patch 1 needs to be reviewed and picked up.
> >
> > v2:
> > * Introduce patch 1 to export device_is_dependent() to modules as needed by
> >   patch 2.
> >
> > Liu Ying (2):
> >   driver core: Export device_is_dependent() to modules
> >   drm/bridge: panel: Check device dependency before managing device link
> 
> I just applied patch 1 directly to the drm-misc-fixes so we don't have to
> revert and then re-apply patches, because that is a bigger evil. (We can't
> rebase these branches...)

Erm, you did wait for GKH or Rafael's ACK to do that, right?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[Linus Walleij]

On Mon, Nov 27, 2023 at 5:29 PM Maxime Ripard <mripard@kernel.org> wrote:
> On Mon, Nov 27, 2023 at 05:03:53PM +0100, Linus Walleij wrote:

> > > Liu Ying (2):
> > >   driver core: Export device_is_dependent() to modules
> > >   drm/bridge: panel: Check device dependency before managing device link
> >
> > I just applied patch 1 directly to the drm-misc-fixes so we don't have to
> > revert and then re-apply patches, because that is a bigger evil. (We can't
> > rebase these branches...)
>
> Erm, you did wait for GKH or Rafael's ACK to do that, right?

No.

It is a bigger evil to leave the tree broken than to enforce formal process,
and it is pretty self-evident. If any of them get annoyed about it we can
revert the patch, or both.

Yours,
Linus Walleij

Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

Hi Linus,

On Mon, Nov 27, 2023 at 11:13:31PM +0100, Linus Walleij wrote:
> On Mon, Nov 27, 2023 at 5:29 PM Maxime Ripard <mripard@kernel.org> wrote:
> > On Mon, Nov 27, 2023 at 05:03:53PM +0100, Linus Walleij wrote:
> 
> > > > Liu Ying (2):
> > > >   driver core: Export device_is_dependent() to modules
> > > >   drm/bridge: panel: Check device dependency before managing device link
> > >
> > > I just applied patch 1 directly to the drm-misc-fixes so we don't have to
> > > revert and then re-apply patches, because that is a bigger evil. (We can't
> > > rebase these branches...)
> >
> > Erm, you did wait for GKH or Rafael's ACK to do that, right?
> 
> No.
> 
> It is a bigger evil to leave the tree broken than to enforce formal process,
> and it is pretty self-evident. If any of them get annoyed about it we can
> revert the patch, or both.

Yeah, I definitely understand why you did it, but I don't think it's
something we would encourage in drm-misc.

We've discussed it with Sima yesterday, and I think we would even need
the extra check in dim to make sure that a committer shouldn't do that
without dim complaining.

Sima played a bit with it, and it should be doable to get something
fairly reliable if you use get_maintainers.pl to retrieve the git tree
(through scripts/get_maintainer.pl --no-email --no-l --scm) and figuring
out if only drm.git, drm-intel.git or drm-misc.git is involved.

If it isn't, then we should check that there's the ack of one of the
maintainers.

Could you write a patch to do so?

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[Linus Walleij]

On Wed, Nov 29, 2023 at 1:32 PM Maxime Ripard <mripard@kernel.org> wrote:
[Me]
> > It is a bigger evil to leave the tree broken than to enforce formal process,
> > and it is pretty self-evident. If any of them get annoyed about it we can
> > revert the patch, or both.
>
> Yeah, I definitely understand why you did it, but I don't think it's
> something we would encourage in drm-misc.

Hm OK I guess, it can be debated but no point in debating it either.

> We've discussed it with Sima yesterday, and I think we would even need
> the extra check in dim to make sure that a committer shouldn't do that
> without dim complaining.
(...)
> Sima played a bit with it, and it should be doable to get something
> fairly reliable if you use get_maintainers.pl to retrieve the git tree
> (through scripts/get_maintainer.pl --no-email --no-l --scm) and figuring
> out if only drm.git, drm-intel.git or drm-misc.git is involved.
>
> If it isn't, then we should check that there's the ack of one of the
> maintainers.

So check for any code that is hitting namespaces outside drivers/gpu/*
Documentation/gpu/* or include/[uapi/]drm/* that it is ACKed by the respective
maintainer for that area of the kernel?

> Could you write a patch to do so?

Patch dim? Well my bash skills are a bit so-so. But I guess I could
learn it. But did you say there is already a prototype?

Yours,
Linus Walleij

Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2004 bytes --]

Hi Linus,

On Wed, Nov 29, 2023 at 03:38:35PM +0100, Linus Walleij wrote:
> On Wed, Nov 29, 2023 at 1:32 PM Maxime Ripard <mripard@kernel.org> wrote:
> [Me]
> > > It is a bigger evil to leave the tree broken than to enforce formal process,
> > > and it is pretty self-evident. If any of them get annoyed about it we can
> > > revert the patch, or both.
> >
> > Yeah, I definitely understand why you did it, but I don't think it's
> > something we would encourage in drm-misc.
> 
> Hm OK I guess, it can be debated but no point in debating it either.
> 
> > We've discussed it with Sima yesterday, and I think we would even need
> > the extra check in dim to make sure that a committer shouldn't do that
> > without dim complaining.
> (...)
> > Sima played a bit with it, and it should be doable to get something
> > fairly reliable if you use get_maintainers.pl to retrieve the git tree
> > (through scripts/get_maintainer.pl --no-email --no-l --scm) and figuring
> > out if only drm.git, drm-intel.git or drm-misc.git is involved.
> >
> > If it isn't, then we should check that there's the ack of one of the
> > maintainers.
> 
> So check for any code that is hitting namespaces outside drivers/gpu/*
> Documentation/gpu/* or include/[uapi/]drm/* that it is ACKed by the respective
> maintainer for that area of the kernel?

We can have something more reliable if we just check the git tree listed
in MAINTAINERS (and returned by get_maintainers --scm). That way we
don't have to whitelist anything, and it will always by in sync with
MAINTAINERS.

And if it's not one of drm trees, then it requires an ack from someone
else get_maintainers will also tell you about.

> > Could you write a patch to do so?
> 
> Patch dim? Well my bash skills are a bit so-so. But I guess I could
> learn it. But did you say there is already a prototype?

My shell skills are also fairly limited, so we just discussed the
solution but didn't do a prototype yet :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]