All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh+dt@kernel.org>
To: Guillaume Tucker <guillaume.tucker@collabora.com>
Cc: "Neil Armstrong" <narmstrong@baylibre.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Sjoerd Simons" <sjoerd.simons@collabora.com>,
	Wookey <wookey@wookware.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	"John Reitan" <john.reitan@arm.com>,
	"Enric Balletbo i Serra" <enric.balletbo@collabora.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
Date: Mon, 24 Apr 2017 08:02:39 -0500	[thread overview]
Message-ID: <CAL_Jsq+G46_hYV9s+emxUGPjJSFDMEhsvZFDu_gs7gK_nC34kw@mail.gmail.com> (raw)
In-Reply-To: <92e0492a-bd17-0171-10a3-8af5044ba6cc@collabora.com>

On Wed, Apr 12, 2017 at 4:36 AM, Guillaume Tucker
<guillaume.tucker@collabora.com> wrote:
> Hi Neil,
>
>
> On 12/04/17 09:48, Neil Armstrong wrote:
>>
>> Hi Guillaume,
>>
>> On 04/12/2017 10:25 AM, Guillaume Tucker wrote:
>>>
>>> Hi Heiko,
>>>
>>> On 11/04/17 21:52, Heiko Stübner wrote:
>>>>
>>>> Hi Guillaume,
>>>>
>>>> Am Dienstag, 11. April 2017, 18:40:37 CEST schrieb Guillaume Tucker:
>>>>>
>>>>> On 03/04/17 09:12, Neil Armstrong wrote:
>>>>>>
>>>>>> On 04/02/2017 09:59 AM, Guillaume Tucker wrote:
>>>>>>>
>>>>>>> +Optional:
>>>>>>> +
>>>>>>> +- clocks : Phandle to clock for the Mali Midgard device.
>>>>>>> +- clock-names : Shall be "clk_mali".
>>>>>>> +- mali-supply : Phandle to regulator for the Mali device. Refer to
>>>>>>> +  Documentation/devicetree/bindings/regulator/regulator.txt for
>>>>>>> details.
>>>>>>> +- operating-points : Refer to
>>>>>>> Documentation/devicetree/bindings/power/opp.txt +  for details.
>>>>>>
>>>>>>
>>>>>> Please add :
>>>>>>    * Must be one of the following:
>>>>>>       "arm,mali-t820"
>>>>>>
>>>>>>    * And, optionally, one of the vendor specific compatible:
>>>>>>       "amlogic,meson-gxm-mali"
>>>>>>
>>>>>> with my Ack for the amlogic platform.
>>>>>
>>>>>
>>>>> It seems to me that as long as the GPU architecture hasn't been
>>>>> modified (I don't think I've ever encountered such a case) then
>>>>> it has to be a standard ARM Mali type regardless of the SoC
>>>>> vendor.  So unless a Mali-T820 in the Amlogic S912 SoC is not the
>>>>> same as a T820 in a different SoC, please forgive me but I don't
>>>>> understand why a vendor compatible string is needed.  My main
>>>>> concern is that it's going to be very hard to keep that list
>>>>> up-to-date with all existing Midgard SoC variants.  If do we need
>>>>> to add vendor compatible strings to correctly describe the
>>>>> hardware then I'm happy to add the amlogic one in my patch v3; I
>>>>> would just like to understand why that's necessary.
>>>>
>>>>
>>>> SoC vendors in most cases hook ip blocks into their socs in different
>>>> and often strange ways. After all it's not some discrete ic you solder
>>>> onto a board, but instead a part of the soc itself.
>>>
>>>
>>> Thanks for your explanation.  I see, it's really about special
>>> things that are not supported by the standard Midgard kernel
>>> driver.
>>>
>>>> So in most cases you will have some hooks outside the actual gpu iospace
>>>> that can be used to tune different things about how the gpu interacts
>>>> with
>>>> the system. Which is probably also the reason the midgard kernel driver
>>>> has this ugly "platform" subdirectory for compile-time platform
>>>> selection.
>>>
>>>
>>> I see the "platform" directory approach as an old and deprecated
>>> way of supporting platforms, upstreaming the dt bindings goes in
>>> the direction of using solely the device tree to describe the GPU
>>> hardware (i.e. CONFIG_MALI_PLATFORM_DEVICETREE).  If something
>>> quirky is needed in the platform, it should be possible to
>>> support it outside the GPU driver (platform devfreq etc...).
>>
>>
>> If this was so simple...
>>
>> This is why the "vendor" compatible is optional.
>>
>> And on another side, the binding were written by ARM, are may not be
>> compatible with how the mainline Linux handles these uses-cases.
>>
>> ARM added some tweaks to handle some weird integration using DT
>> properties,
>> but this should definitely go in platform specific code instead.
>
>
> OK, sorry I was approaching the issue from a completely different
> and somewhat more idealistic angle.  My impression is that if the
> driver was in mainline then it would be maintained in such a way
> that vendor compatible strings would not be required, but this is
> all hypothetical.
>
> So in practice, I think I now better understand why vendor
> compatible strings may still be needed.  And they're optional, so
> harmless to other platforms, so it's all fine with me :)

SoC specific compatibles are required. They are only optional for a
driver to use.

Rob

WARNING: multiple messages have this Message-ID
From: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Guillaume Tucker
	<guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Cc: "Neil Armstrong"
	<narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>,
	"Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>,
	"Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"Sjoerd Simons"
	<sjoerd.simons-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
	Wookey <wookey-Xpnwy/r4z8dg9hUCZPvPmw@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"John Reitan" <john.reitan-5wv7dgnIgG8@public.gmane.org>,
	"Enric Balletbo i Serra"
	<enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH v2 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
Date: Mon, 24 Apr 2017 08:02:39 -0500	[thread overview]
Message-ID: <CAL_Jsq+G46_hYV9s+emxUGPjJSFDMEhsvZFDu_gs7gK_nC34kw@mail.gmail.com> (raw)
In-Reply-To: <92e0492a-bd17-0171-10a3-8af5044ba6cc-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

On Wed, Apr 12, 2017 at 4:36 AM, Guillaume Tucker
<guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> wrote:
> Hi Neil,
>
>
> On 12/04/17 09:48, Neil Armstrong wrote:
>>
>> Hi Guillaume,
>>
>> On 04/12/2017 10:25 AM, Guillaume Tucker wrote:
>>>
>>> Hi Heiko,
>>>
>>> On 11/04/17 21:52, Heiko Stübner wrote:
>>>>
>>>> Hi Guillaume,
>>>>
>>>> Am Dienstag, 11. April 2017, 18:40:37 CEST schrieb Guillaume Tucker:
>>>>>
>>>>> On 03/04/17 09:12, Neil Armstrong wrote:
>>>>>>
>>>>>> On 04/02/2017 09:59 AM, Guillaume Tucker wrote:
>>>>>>>
>>>>>>> +Optional:
>>>>>>> +
>>>>>>> +- clocks : Phandle to clock for the Mali Midgard device.
>>>>>>> +- clock-names : Shall be "clk_mali".
>>>>>>> +- mali-supply : Phandle to regulator for the Mali device. Refer to
>>>>>>> +  Documentation/devicetree/bindings/regulator/regulator.txt for
>>>>>>> details.
>>>>>>> +- operating-points : Refer to
>>>>>>> Documentation/devicetree/bindings/power/opp.txt +  for details.
>>>>>>
>>>>>>
>>>>>> Please add :
>>>>>>    * Must be one of the following:
>>>>>>       "arm,mali-t820"
>>>>>>
>>>>>>    * And, optionally, one of the vendor specific compatible:
>>>>>>       "amlogic,meson-gxm-mali"
>>>>>>
>>>>>> with my Ack for the amlogic platform.
>>>>>
>>>>>
>>>>> It seems to me that as long as the GPU architecture hasn't been
>>>>> modified (I don't think I've ever encountered such a case) then
>>>>> it has to be a standard ARM Mali type regardless of the SoC
>>>>> vendor.  So unless a Mali-T820 in the Amlogic S912 SoC is not the
>>>>> same as a T820 in a different SoC, please forgive me but I don't
>>>>> understand why a vendor compatible string is needed.  My main
>>>>> concern is that it's going to be very hard to keep that list
>>>>> up-to-date with all existing Midgard SoC variants.  If do we need
>>>>> to add vendor compatible strings to correctly describe the
>>>>> hardware then I'm happy to add the amlogic one in my patch v3; I
>>>>> would just like to understand why that's necessary.
>>>>
>>>>
>>>> SoC vendors in most cases hook ip blocks into their socs in different
>>>> and often strange ways. After all it's not some discrete ic you solder
>>>> onto a board, but instead a part of the soc itself.
>>>
>>>
>>> Thanks for your explanation.  I see, it's really about special
>>> things that are not supported by the standard Midgard kernel
>>> driver.
>>>
>>>> So in most cases you will have some hooks outside the actual gpu iospace
>>>> that can be used to tune different things about how the gpu interacts
>>>> with
>>>> the system. Which is probably also the reason the midgard kernel driver
>>>> has this ugly "platform" subdirectory for compile-time platform
>>>> selection.
>>>
>>>
>>> I see the "platform" directory approach as an old and deprecated
>>> way of supporting platforms, upstreaming the dt bindings goes in
>>> the direction of using solely the device tree to describe the GPU
>>> hardware (i.e. CONFIG_MALI_PLATFORM_DEVICETREE).  If something
>>> quirky is needed in the platform, it should be possible to
>>> support it outside the GPU driver (platform devfreq etc...).
>>
>>
>> If this was so simple...
>>
>> This is why the "vendor" compatible is optional.
>>
>> And on another side, the binding were written by ARM, are may not be
>> compatible with how the mainline Linux handles these uses-cases.
>>
>> ARM added some tweaks to handle some weird integration using DT
>> properties,
>> but this should definitely go in platform specific code instead.
>
>
> OK, sorry I was approaching the issue from a completely different
> and somewhat more idealistic angle.  My impression is that if the
> driver was in mainline then it would be maintained in such a way
> that vendor compatible strings would not be required, but this is
> all hypothetical.
>
> So in practice, I think I now better understand why vendor
> compatible strings may still be needed.  And they're optional, so
> harmless to other platforms, so it's all fine with me :)

SoC specific compatibles are required. They are only optional for a
driver to use.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID
From: robh+dt@kernel.org (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
Date: Mon, 24 Apr 2017 08:02:39 -0500	[thread overview]
Message-ID: <CAL_Jsq+G46_hYV9s+emxUGPjJSFDMEhsvZFDu_gs7gK_nC34kw@mail.gmail.com> (raw)
In-Reply-To: <92e0492a-bd17-0171-10a3-8af5044ba6cc@collabora.com>

On Wed, Apr 12, 2017 at 4:36 AM, Guillaume Tucker
<guillaume.tucker@collabora.com> wrote:
> Hi Neil,
>
>
> On 12/04/17 09:48, Neil Armstrong wrote:
>>
>> Hi Guillaume,
>>
>> On 04/12/2017 10:25 AM, Guillaume Tucker wrote:
>>>
>>> Hi Heiko,
>>>
>>> On 11/04/17 21:52, Heiko St?bner wrote:
>>>>
>>>> Hi Guillaume,
>>>>
>>>> Am Dienstag, 11. April 2017, 18:40:37 CEST schrieb Guillaume Tucker:
>>>>>
>>>>> On 03/04/17 09:12, Neil Armstrong wrote:
>>>>>>
>>>>>> On 04/02/2017 09:59 AM, Guillaume Tucker wrote:
>>>>>>>
>>>>>>> +Optional:
>>>>>>> +
>>>>>>> +- clocks : Phandle to clock for the Mali Midgard device.
>>>>>>> +- clock-names : Shall be "clk_mali".
>>>>>>> +- mali-supply : Phandle to regulator for the Mali device. Refer to
>>>>>>> +  Documentation/devicetree/bindings/regulator/regulator.txt for
>>>>>>> details.
>>>>>>> +- operating-points : Refer to
>>>>>>> Documentation/devicetree/bindings/power/opp.txt +  for details.
>>>>>>
>>>>>>
>>>>>> Please add :
>>>>>>    * Must be one of the following:
>>>>>>       "arm,mali-t820"
>>>>>>
>>>>>>    * And, optionally, one of the vendor specific compatible:
>>>>>>       "amlogic,meson-gxm-mali"
>>>>>>
>>>>>> with my Ack for the amlogic platform.
>>>>>
>>>>>
>>>>> It seems to me that as long as the GPU architecture hasn't been
>>>>> modified (I don't think I've ever encountered such a case) then
>>>>> it has to be a standard ARM Mali type regardless of the SoC
>>>>> vendor.  So unless a Mali-T820 in the Amlogic S912 SoC is not the
>>>>> same as a T820 in a different SoC, please forgive me but I don't
>>>>> understand why a vendor compatible string is needed.  My main
>>>>> concern is that it's going to be very hard to keep that list
>>>>> up-to-date with all existing Midgard SoC variants.  If do we need
>>>>> to add vendor compatible strings to correctly describe the
>>>>> hardware then I'm happy to add the amlogic one in my patch v3; I
>>>>> would just like to understand why that's necessary.
>>>>
>>>>
>>>> SoC vendors in most cases hook ip blocks into their socs in different
>>>> and often strange ways. After all it's not some discrete ic you solder
>>>> onto a board, but instead a part of the soc itself.
>>>
>>>
>>> Thanks for your explanation.  I see, it's really about special
>>> things that are not supported by the standard Midgard kernel
>>> driver.
>>>
>>>> So in most cases you will have some hooks outside the actual gpu iospace
>>>> that can be used to tune different things about how the gpu interacts
>>>> with
>>>> the system. Which is probably also the reason the midgard kernel driver
>>>> has this ugly "platform" subdirectory for compile-time platform
>>>> selection.
>>>
>>>
>>> I see the "platform" directory approach as an old and deprecated
>>> way of supporting platforms, upstreaming the dt bindings goes in
>>> the direction of using solely the device tree to describe the GPU
>>> hardware (i.e. CONFIG_MALI_PLATFORM_DEVICETREE).  If something
>>> quirky is needed in the platform, it should be possible to
>>> support it outside the GPU driver (platform devfreq etc...).
>>
>>
>> If this was so simple...
>>
>> This is why the "vendor" compatible is optional.
>>
>> And on another side, the binding were written by ARM, are may not be
>> compatible with how the mainline Linux handles these uses-cases.
>>
>> ARM added some tweaks to handle some weird integration using DT
>> properties,
>> but this should definitely go in platform specific code instead.
>
>
> OK, sorry I was approaching the issue from a completely different
> and somewhat more idealistic angle.  My impression is that if the
> driver was in mainline then it would be maintained in such a way
> that vendor compatible strings would not be required, but this is
> all hypothetical.
>
> So in practice, I think I now better understand why vendor
> compatible strings may still be needed.  And they're optional, so
> harmless to other platforms, so it's all fine with me :)

SoC specific compatibles are required. They are only optional for a
driver to use.

Rob

  reply	other threads:[~2017-04-24 13:03 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-02  7:59 [PATCH v2 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288 Guillaume Tucker
2017-04-02  7:59 ` Guillaume Tucker
2017-04-02  7:59 ` Guillaume Tucker
2017-04-02  7:59 ` [PATCH v2 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-03  8:12   ` Neil Armstrong
2017-04-03  8:12     ` Neil Armstrong
2017-04-03  8:12     ` Neil Armstrong
2017-04-11 17:40     ` Guillaume Tucker
2017-04-11 17:40       ` Guillaume Tucker
2017-04-11 17:40       ` Guillaume Tucker
2017-04-11 20:52       ` Heiko Stübner
2017-04-11 20:52         ` Heiko Stübner
2017-04-11 20:52         ` Heiko Stübner
2017-04-12  8:25         ` Guillaume Tucker
2017-04-12  8:25           ` Guillaume Tucker
2017-04-12  8:48           ` Neil Armstrong
2017-04-12  8:48             ` Neil Armstrong
2017-04-12  8:48             ` Neil Armstrong
2017-04-12  9:36             ` Guillaume Tucker
2017-04-12  9:36               ` Guillaume Tucker
2017-04-12  9:36               ` Guillaume Tucker
2017-04-24 13:02               ` Rob Herring [this message]
2017-04-24 13:02                 ` Rob Herring
2017-04-24 13:02                 ` Rob Herring
2017-04-04  2:00   ` Rob Herring
2017-04-04  2:00     ` Rob Herring
2017-04-04  2:00     ` Rob Herring
2017-04-18  9:15     ` Guillaume Tucker
2017-04-18  9:15       ` Guillaume Tucker
2017-04-18  9:15       ` Guillaume Tucker
2017-04-24 10:43       ` Guillaume Tucker
2017-04-24 10:43         ` Guillaume Tucker
2017-04-24 10:43         ` Guillaume Tucker
2017-04-02  7:59 ` [PATCH v2 2/5] ARM: dts: rockchip: add ARM Mali GPU node for rk3288 Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-02 10:41   ` Enric Balletbo i Serra
2017-04-02 10:41     ` Enric Balletbo i Serra
2017-04-02  7:59 ` [PATCH v2 3/5] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-rock2-som Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-02  7:59 ` [PATCH v2 4/5] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-firefly Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-02  7:59 ` [PATCH v2 5/5] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-veyron Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-02  7:59   ` Guillaume Tucker
2017-04-03 22:54 ` [PATCH v2 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288 Heiko Stübner
2017-04-03 22:54   ` Heiko Stübner
2017-04-03 22:54   ` Heiko Stübner

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=CAL_Jsq+G46_hYV9s+emxUGPjJSFDMEhsvZFDu_gs7gK_nC34kw@mail.gmail.com \
    --to=robh+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=enric.balletbo@collabora.com \
    --cc=guillaume.tucker@collabora.com \
    --cc=heiko@sntech.de \
    --cc=john.reitan@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=narmstrong@baylibre.com \
    --cc=sjoerd.simons@collabora.com \
    --cc=wookey@wookware.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: link
Be 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.