Re: [PATCH 25/49] staging: hikey9xx/gpu: do some code cleanups

2020-08-21 Thread John Stultz
On Thu, Aug 20, 2020 at 1:23 AM Mauro Carvalho Chehab
 wrote:
>
> (added c/c Rob Herring)
>
> Em Wed, 19 Aug 2020 18:53:06 -0700
> John Stultz  escreveu:
>
> > On Wed, Aug 19, 2020 at 4:46 AM Mauro Carvalho Chehab
> >  wrote:
> > > @@ -376,7 +355,7 @@ static int kirin_drm_platform_resume(struct 
> > > platform_device *pdev)
> > >  }
> > >
> > >  static const struct of_device_id kirin_drm_dt_ids[] = {
> > > -   { .compatible = "hisilicon,hi3660-dpe",
> > > +   { .compatible = "hisilicon,kirin960-dpe",
> >
> >
> > One issue, elsewhere in your patch stack you still refer to the
> > hisilicon,hi3660-dpe compatible string. This should probably be
> > consistent one way or the other.
>
> Agreed with regards to consistency.
>
> It sounds to me that calling those as Kirin 9xx (and the previous one
> as Kirin 620) is better than using the part number.
>
> Here, googling for "Kirin 970" gave about 6.9 million hits, while "Hi3670"
> gave only 75,5K hits.
>
> Kirin 620 has similar results: 6.85 million hits, against 61,9 hits
> for "Hi3620".

Hi6620 is kirin 620 I believe.

> With "Kirin 960", the numbers are a lot higher: had 21,4 million hits,
> against 423K hits for "Hi3660".
>
> So, my preference is to use "Kirin 620, 960 and 970" for future changes.

I think traditionally the DTS is developed with the hardware
documentation sometimes before the SoC is announced, so they tend to
stick with whatever those documents call it, rather than (later more
google-able) marketing names.

That said, I don't have a preference, as long as it's consistent, and
we don't change compatible strings that are already upstream.

> I would love to make this consistent among the Kernel. However,
> I'm not sure if changing "compatible" would be acceptable
> by DT maintainers.
>

Existing bindings are already ABI. So we can't change those. New
bindings can be set to what makes the most sense.

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


Re: [PATCH 25/49] staging: hikey9xx/gpu: do some code cleanups

2020-08-20 Thread Mauro Carvalho Chehab
(added c/c Rob Herring)

Em Wed, 19 Aug 2020 18:53:06 -0700
John Stultz  escreveu:

> On Wed, Aug 19, 2020 at 4:46 AM Mauro Carvalho Chehab
>  wrote:
> > @@ -376,7 +355,7 @@ static int kirin_drm_platform_resume(struct 
> > platform_device *pdev)
> >  }
> >
> >  static const struct of_device_id kirin_drm_dt_ids[] = {
> > -   { .compatible = "hisilicon,hi3660-dpe",
> > +   { .compatible = "hisilicon,kirin960-dpe",  
> 
> 
> One issue, elsewhere in your patch stack you still refer to the
> hisilicon,hi3660-dpe compatible string. This should probably be
> consistent one way or the other.

Agreed with regards to consistency.

It sounds to me that calling those as Kirin 9xx (and the previous one
as Kirin 620) is better than using the part number.

Here, googling for "Kirin 970" gave about 6.9 million hits, while "Hi3670"
gave only 75,5K hits.

Kirin 620 has similar results: 6.85 million hits, against 61,9 hits
for "Hi3620".

With "Kirin 960", the numbers are a lot higher: had 21,4 million hits,
against 423K hits for "Hi3660".

So, my preference is to use "Kirin 620, 960 and 970" for future changes.

-

Currently, there are already some inconsistency, as some places
use the part number where others use "Kirin xxx" designation,
when referring to Kirin 620, 960 and 970.

I would love to make this consistent among the Kernel. However,
I'm not sure if changing "compatible" would be acceptable
by DT maintainers.

If something like that would be OK, I can prepare a separate
patchset to be applied at the Kernel.

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


Re: [PATCH 25/49] staging: hikey9xx/gpu: do some code cleanups

2020-08-19 Thread John Stultz
On Wed, Aug 19, 2020 at 4:46 AM Mauro Carvalho Chehab
 wrote:
> @@ -376,7 +355,7 @@ static int kirin_drm_platform_resume(struct 
> platform_device *pdev)
>  }
>
>  static const struct of_device_id kirin_drm_dt_ids[] = {
> -   { .compatible = "hisilicon,hi3660-dpe",
> +   { .compatible = "hisilicon,kirin960-dpe",


One issue, elsewhere in your patch stack you still refer to the
hisilicon,hi3660-dpe compatible string. This should probably be
consistent one way or the other.

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