Re: [linux-sunxi] Re: [PATCH v2 14/27] drm/sun4i: Don't check for panel or bridge on TV TCONs

2018-06-14 Thread Jernej Škrabec
Dne sreda, 13. junij 2018 ob 10:04:20 CEST je Chen-Yu Tsai napisal(a):
> On Wed, Jun 13, 2018 at 3:46 PM, Maxime Ripard
> 
>  wrote:
> > On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote:
> >> TV TCONs are always connected to TV or HDMI encoder, so it doesn't make
> >> sense to check if panel or bridge is connected to them.
> >> 
> >> Check if TCON has channel 0 and only then check for connected panel or
> >> bridges.
> >> 
> >> Signed-off-by: Jernej Skrabec 
> >> ---
> >> 
> >>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +---
> >>  1 file changed, 9 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> >> b/drivers/gpu/drm/sun4i/sun4i_tcon.c index b1205a7bc20f..c9ffa5381185
> >> 100644
> >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> >> @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops
> >> = {>> 
> >>  static int sun4i_tcon_probe(struct platform_device *pdev)
> >>  {
> >>  
> >>   struct device_node *node = pdev->dev.of_node;
> >> 
> >> + const struct sun4i_tcon_quirks *quirks;
> >> 
> >>   struct drm_bridge *bridge;
> >>   struct drm_panel *panel;
> >>   int ret;
> >> 
> >> - ret = drm_of_find_panel_or_bridge(node, 1, 0, , );
> >> - if (ret == -EPROBE_DEFER)
> >> - return ret;
> >> + quirks = of_device_get_match_data(>dev);
> > 
> > We should probably check ofr the pointer value before dereferencing it.
> 
> I think we've discussed this before. If the driver has data structures
> for all the supported compatible strings, and it is device tree only,
> then we should just let it blow up in the user's face, since they are
> obviously doing something they shouldn't be doing to get the driver
> to probe without a compatible string match.

TCON can't work with no quirks specified, since that would mean that neither 
channels are present. Additionally, sun4i_tcon_bind() also doesn't check if 
quirks are NULL or not. So I concur with Chen-Yu here.

Best regards,
Jernej



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 14/27] drm/sun4i: Don't check for panel or bridge on TV TCONs

2018-06-13 Thread Chen-Yu Tsai
On Wed, Jun 13, 2018 at 3:46 PM, Maxime Ripard
 wrote:
> On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote:
>> TV TCONs are always connected to TV or HDMI encoder, so it doesn't make
>> sense to check if panel or bridge is connected to them.
>>
>> Check if TCON has channel 0 and only then check for connected panel or
>> bridges.
>>
>> Signed-off-by: Jernej Skrabec 
>> ---
>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index b1205a7bc20f..c9ffa5381185 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = {
>>  static int sun4i_tcon_probe(struct platform_device *pdev)
>>  {
>>   struct device_node *node = pdev->dev.of_node;
>> + const struct sun4i_tcon_quirks *quirks;
>>   struct drm_bridge *bridge;
>>   struct drm_panel *panel;
>>   int ret;
>>
>> - ret = drm_of_find_panel_or_bridge(node, 1, 0, , );
>> - if (ret == -EPROBE_DEFER)
>> - return ret;
>> + quirks = of_device_get_match_data(>dev);
>
> We should probably check ofr the pointer value before dereferencing it.

I think we've discussed this before. If the driver has data structures
for all the supported compatible strings, and it is device tree only,
then we should just let it blow up in the user's face, since they are
obviously doing something they shouldn't be doing to get the driver
to probe without a compatible string match.

ChenYu
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 14/27] drm/sun4i: Don't check for panel or bridge on TV TCONs

2018-06-13 Thread Maxime Ripard
On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote:
> TV TCONs are always connected to TV or HDMI encoder, so it doesn't make
> sense to check if panel or bridge is connected to them.
> 
> Check if TCON has channel 0 and only then check for connected panel or
> bridges.
> 
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index b1205a7bc20f..c9ffa5381185 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = {
>  static int sun4i_tcon_probe(struct platform_device *pdev)
>  {
>   struct device_node *node = pdev->dev.of_node;
> + const struct sun4i_tcon_quirks *quirks;
>   struct drm_bridge *bridge;
>   struct drm_panel *panel;
>   int ret;
>  
> - ret = drm_of_find_panel_or_bridge(node, 1, 0, , );
> - if (ret == -EPROBE_DEFER)
> - return ret;
> + quirks = of_device_get_match_data(>dev);

We should probably check ofr the pointer value before dereferencing it.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 14/27] drm/sun4i: Don't check for panel or bridge on TV TCONs

2018-06-13 Thread Jernej Skrabec
TV TCONs are always connected to TV or HDMI encoder, so it doesn't make
sense to check if panel or bridge is connected to them.

Check if TCON has channel 0 and only then check for connected panel or
bridges.

Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index b1205a7bc20f..c9ffa5381185 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = {
 static int sun4i_tcon_probe(struct platform_device *pdev)
 {
struct device_node *node = pdev->dev.of_node;
+   const struct sun4i_tcon_quirks *quirks;
struct drm_bridge *bridge;
struct drm_panel *panel;
int ret;
 
-   ret = drm_of_find_panel_or_bridge(node, 1, 0, , );
-   if (ret == -EPROBE_DEFER)
-   return ret;
+   quirks = of_device_get_match_data(>dev);
+
+   /* panels and bridges are present only on TCONs with channel 0 */
+   if (quirks->has_channel_0) {
+   ret = drm_of_find_panel_or_bridge(node, 1, 0, , );
+   if (ret == -EPROBE_DEFER)
+   return ret;
+   }
 
return component_add(>dev, _tcon_ops);
 }
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel