Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
On 05/07/2019 03:35, CK Hu wrote: > Hi, Uli: > > On Thu, 2019-07-04 at 17:33 +0200, Ulrich Hecht wrote: >>> On July 4, 2019 at 11:08 AM Matthias Brugger wrote: >>> You are right, it took far too long for me to respond with a new version of >>> the >>> series. The problem I face is, that I use my mt8173 based chromebook for >>> testing. It needs some downstream patches and broke somewhere between my >>> last >>> email and a few month ago. >> >> If that Chromebook is an Acer R13 and you need a working kernel, you may >> want to have a look at https://github.com/uli/kernel/tree/elm-working-5.2 . > > Thanks for your sample code, and your implementation is different than > Matthias' version. In your version, mmsys is a single device which has > clock function and display function, the clock function is placed in > clock driver folder and display function is placed in drm driver folder. > In Matthias' version, clock function is a sub device of mmsys. I've no > idea of which one is better. I would get more information to make better > decision. > From the discussion we had here and the last comments from Stephen I thin what we should do is: - create a platform driver for the mmsys clock part, in the drivers/clk/mediatek/... - the DRM driver creates a platform device which will probe the clock driver. This way you won't need to change any compatible. You will have to re-read the discussions in the different versions of this series. My first approach was to usa a mfd driver for mmsys, which was rejected. The last approach was to create a DTS sub node, also rejected :) Regards, Matthias > Regards, > CK > >> >> CU >> Uli >> >> ___ >> Linux-mediatek mailing list >> linux-media...@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-mediatek > > >
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
Hi, Uli: On Thu, 2019-07-04 at 17:33 +0200, Ulrich Hecht wrote: > > On July 4, 2019 at 11:08 AM Matthias Brugger wrote: > > You are right, it took far too long for me to respond with a new version of > > the > > series. The problem I face is, that I use my mt8173 based chromebook for > > testing. It needs some downstream patches and broke somewhere between my > > last > > email and a few month ago. > > If that Chromebook is an Acer R13 and you need a working kernel, you may want > to have a look at https://github.com/uli/kernel/tree/elm-working-5.2 . Thanks for your sample code, and your implementation is different than Matthias' version. In your version, mmsys is a single device which has clock function and display function, the clock function is placed in clock driver folder and display function is placed in drm driver folder. In Matthias' version, clock function is a sub device of mmsys. I've no idea of which one is better. I would get more information to make better decision. Regards, CK > > CU > Uli > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
> On July 4, 2019 at 11:08 AM Matthias Brugger wrote: > You are right, it took far too long for me to respond with a new version of > the > series. The problem I face is, that I use my mt8173 based chromebook for > testing. It needs some downstream patches and broke somewhere between my last > email and a few month ago. If that Chromebook is an Acer R13 and you need a working kernel, you may want to have a look at https://github.com/uli/kernel/tree/elm-working-5.2 . CU Uli
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
Hi CK-Hu,
On 01/07/2019 05:55, CK Hu wrote:
> Hi, Matthias:
>
> On Fri, 2018-11-30 at 16:59 +0800, Matthias Brugger wrote:
>>
>> On 30/11/2018 07:43, Stephen Boyd wrote:
>>> Quoting Matthias Brugger (2018-11-21 09:09:52)
On 21/11/2018 17:46, Stephen Boyd wrote:
> Quoting Rob Herring (2018-11-19 11:15:16)
>> On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
>> wrote:
>>> On 11/17/18 12:15 AM, Rob Herring wrote:
On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org
wrote:
> -#clock-cells = <1>;
> +
> +mmsys_clk: clock-controller@1400 {
> +compatible = "mediatek,mt2712-mmsys-clk";
> +#clock-cells = <1>;
This goes against the general direction of not defining separate nodes
for providers with no resources.
Why do you need this and what does it buy if you have to continue to
support the existing chips?
>>>
>>> It would show explicitly that the mmsys block is used to probe two
>>> drivers, one for the gpu and one for the clocks. Otherwise that is
>>> hidden in the drm driver code. I think it is cleaner to describe that in
>>> the device tree.
>>
>> No, that's maybe cleaner for the driver implementation in the Linux
>> kernel. What about other OS's or when Linux drivers and subsystems
>> needs change? Cleaner for DT is design bindings that reflect the h/w.
>> Hardware is sometimes just messy.
>>
>
> I agree. I fail to see what this patch series is doing besides changing
> driver probe and device creation methods and making a backwards
> incompatible change to DT. Is there any other benefit here?
>
You are referring whole series?
Citing the cover letter:
"MMSYS in Mediatek SoCs has some registers to control clock gates (which is
used in the clk driver) and some registers to set the routing and enable
the differnet (sic!) blocks of the display subsystem.
Up to now both drivers, clock and drm are probed with the same device tree
compatible. But only the first driver get probed, which in effect breaks
graphics on mt8173 and mt2701.
>>>
>>> Ouch!
>>>
>>
>> Yes :)
>>
This patch uses a platform device registration in the DRM driver, which
will trigger the probe of the corresponding clock driver. It was tested on
the
bananapi-r2 and the Acer R13 Chromebook."
>>>
>>> Alright, please don't add nodes in DT just to make device drivers probe.
>>> Instead, register clks from the drm driver or create a child platform
>>> device for the clk bits purely in the drm driver and have that probe the
>>> associated clk driver from there.
>>>
>>
>> I'll make the other SoCs probe via a child platform device from the drm
>> driver,
>> as already done in 2/12 and 3/12.
>
> This series have been pending for half an year, would you keep going on
> this series? If you're busy, I could complete this series, but I need to
> know what you have plan to do.
>
You are right, it took far too long for me to respond with a new version of the
series. The problem I face is, that I use my mt8173 based chromebook for
testing. It needs some downstream patches and broke somewhere between my last
email and a few month ago. I wasn't able to get serial console to work, which
made things even more complicated. Anyway, long story short, I got sidetracked
with other stuff and didn't send a new version.
If you have time to work on this, I'd happy to see things being pushed forward
by you :)
> I guess that 1/12 ~ 5/12 is for MT2701/MT8173 and that patches meet this
> discussion. 6/12 ~ 12/12 is for MT2712/MT6797 but that patches does not
> meet this discussion. So the unfinished work is to make MT2712/MT6797 to
> align MT2701/MT8173, is this right?
After re-reading the emails I think the missing part is, to probe the clocks
from the DRM driver instead of adding a new devicetree binding for them.
Regards,
Matthias
>
> Regards,
> CK
>
>>
>> Regards,
>> Matthias
>>
DT is broken right now, because two drivers rely on the same node, which
gets
consumed just once. The new DT introduced does not break anything because
it is
only used for boards that: "[..] are not available to the general public
(mt2712e) or only have the mmsys clock driver part implemented (mt6797)."
>>>
>>> Ok, so backwards compatibility is irrelevant then. Sounds fine to me.
>>>
>>>
>>
>> ___
>> Linux-mediatek mailing list
>> linux-media...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
Hi, Matthias:
On Fri, 2018-11-30 at 16:59 +0800, Matthias Brugger wrote:
>
> On 30/11/2018 07:43, Stephen Boyd wrote:
> > Quoting Matthias Brugger (2018-11-21 09:09:52)
> >>
> >>
> >> On 21/11/2018 17:46, Stephen Boyd wrote:
> >>> Quoting Rob Herring (2018-11-19 11:15:16)
> On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
> wrote:
> > On 11/17/18 12:15 AM, Rob Herring wrote:
> >> On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org
> >> wrote:
> >>> -#clock-cells = <1>;
> >>> +
> >>> +mmsys_clk: clock-controller@1400 {
> >>> +compatible = "mediatek,mt2712-mmsys-clk";
> >>> +#clock-cells = <1>;
> >>
> >> This goes against the general direction of not defining separate nodes
> >> for providers with no resources.
> >>
> >> Why do you need this and what does it buy if you have to continue to
> >> support the existing chips?
> >>
> >
> > It would show explicitly that the mmsys block is used to probe two
> > drivers, one for the gpu and one for the clocks. Otherwise that is
> > hidden in the drm driver code. I think it is cleaner to describe that in
> > the device tree.
>
> No, that's maybe cleaner for the driver implementation in the Linux
> kernel. What about other OS's or when Linux drivers and subsystems
> needs change? Cleaner for DT is design bindings that reflect the h/w.
> Hardware is sometimes just messy.
>
> >>>
> >>> I agree. I fail to see what this patch series is doing besides changing
> >>> driver probe and device creation methods and making a backwards
> >>> incompatible change to DT. Is there any other benefit here?
> >>>
> >>
> >> You are referring whole series?
> >> Citing the cover letter:
> >> "MMSYS in Mediatek SoCs has some registers to control clock gates (which is
> >> used in the clk driver) and some registers to set the routing and enable
> >> the differnet (sic!) blocks of the display subsystem.
> >>
> >> Up to now both drivers, clock and drm are probed with the same device tree
> >> compatible. But only the first driver get probed, which in effect breaks
> >> graphics on mt8173 and mt2701.
> >
> > Ouch!
> >
>
> Yes :)
>
> >>
> >> This patch uses a platform device registration in the DRM driver, which
> >> will trigger the probe of the corresponding clock driver. It was tested on
> >> the
> >> bananapi-r2 and the Acer R13 Chromebook."
> >
> > Alright, please don't add nodes in DT just to make device drivers probe.
> > Instead, register clks from the drm driver or create a child platform
> > device for the clk bits purely in the drm driver and have that probe the
> > associated clk driver from there.
> >
>
> I'll make the other SoCs probe via a child platform device from the drm
> driver,
> as already done in 2/12 and 3/12.
This series have been pending for half an year, would you keep going on
this series? If you're busy, I could complete this series, but I need to
know what you have plan to do.
I guess that 1/12 ~ 5/12 is for MT2701/MT8173 and that patches meet this
discussion. 6/12 ~ 12/12 is for MT2712/MT6797 but that patches does not
meet this discussion. So the unfinished work is to make MT2712/MT6797 to
align MT2701/MT8173, is this right?
Regards,
CK
>
> Regards,
> Matthias
>
> >>
> >> DT is broken right now, because two drivers rely on the same node, which
> >> gets
> >> consumed just once. The new DT introduced does not break anything because
> >> it is
> >> only used for boards that: "[..] are not available to the general public
> >> (mt2712e) or only have the mmsys clock driver part implemented (mt6797)."
> >
> > Ok, so backwards compatibility is irrelevant then. Sounds fine to me.
> >
> >
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
On 30/11/2018 07:43, Stephen Boyd wrote:
> Quoting Matthias Brugger (2018-11-21 09:09:52)
>>
>>
>> On 21/11/2018 17:46, Stephen Boyd wrote:
>>> Quoting Rob Herring (2018-11-19 11:15:16)
On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
wrote:
> On 11/17/18 12:15 AM, Rob Herring wrote:
>> On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
>>> -#clock-cells = <1>;
>>> +
>>> +mmsys_clk: clock-controller@1400 {
>>> +compatible = "mediatek,mt2712-mmsys-clk";
>>> +#clock-cells = <1>;
>>
>> This goes against the general direction of not defining separate nodes
>> for providers with no resources.
>>
>> Why do you need this and what does it buy if you have to continue to
>> support the existing chips?
>>
>
> It would show explicitly that the mmsys block is used to probe two
> drivers, one for the gpu and one for the clocks. Otherwise that is
> hidden in the drm driver code. I think it is cleaner to describe that in
> the device tree.
No, that's maybe cleaner for the driver implementation in the Linux
kernel. What about other OS's or when Linux drivers and subsystems
needs change? Cleaner for DT is design bindings that reflect the h/w.
Hardware is sometimes just messy.
>>>
>>> I agree. I fail to see what this patch series is doing besides changing
>>> driver probe and device creation methods and making a backwards
>>> incompatible change to DT. Is there any other benefit here?
>>>
>>
>> You are referring whole series?
>> Citing the cover letter:
>> "MMSYS in Mediatek SoCs has some registers to control clock gates (which is
>> used in the clk driver) and some registers to set the routing and enable
>> the differnet (sic!) blocks of the display subsystem.
>>
>> Up to now both drivers, clock and drm are probed with the same device tree
>> compatible. But only the first driver get probed, which in effect breaks
>> graphics on mt8173 and mt2701.
>
> Ouch!
>
Yes :)
>>
>> This patch uses a platform device registration in the DRM driver, which
>> will trigger the probe of the corresponding clock driver. It was tested on
>> the
>> bananapi-r2 and the Acer R13 Chromebook."
>
> Alright, please don't add nodes in DT just to make device drivers probe.
> Instead, register clks from the drm driver or create a child platform
> device for the clk bits purely in the drm driver and have that probe the
> associated clk driver from there.
>
I'll make the other SoCs probe via a child platform device from the drm driver,
as already done in 2/12 and 3/12.
Regards,
Matthias
>>
>> DT is broken right now, because two drivers rely on the same node, which gets
>> consumed just once. The new DT introduced does not break anything because it
>> is
>> only used for boards that: "[..] are not available to the general public
>> (mt2712e) or only have the mmsys clock driver part implemented (mt6797)."
>
> Ok, so backwards compatibility is irrelevant then. Sounds fine to me.
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
Quoting Matthias Brugger (2018-11-21 09:09:52)
>
>
> On 21/11/2018 17:46, Stephen Boyd wrote:
> > Quoting Rob Herring (2018-11-19 11:15:16)
> >> On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
> >> wrote:
> >>> On 11/17/18 12:15 AM, Rob Herring wrote:
> On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
> > -#clock-cells = <1>;
> > +
> > +mmsys_clk: clock-controller@1400 {
> > +compatible = "mediatek,mt2712-mmsys-clk";
> > +#clock-cells = <1>;
>
> This goes against the general direction of not defining separate nodes
> for providers with no resources.
>
> Why do you need this and what does it buy if you have to continue to
> support the existing chips?
>
> >>>
> >>> It would show explicitly that the mmsys block is used to probe two
> >>> drivers, one for the gpu and one for the clocks. Otherwise that is
> >>> hidden in the drm driver code. I think it is cleaner to describe that in
> >>> the device tree.
> >>
> >> No, that's maybe cleaner for the driver implementation in the Linux
> >> kernel. What about other OS's or when Linux drivers and subsystems
> >> needs change? Cleaner for DT is design bindings that reflect the h/w.
> >> Hardware is sometimes just messy.
> >>
> >
> > I agree. I fail to see what this patch series is doing besides changing
> > driver probe and device creation methods and making a backwards
> > incompatible change to DT. Is there any other benefit here?
> >
>
> You are referring whole series?
> Citing the cover letter:
> "MMSYS in Mediatek SoCs has some registers to control clock gates (which is
> used in the clk driver) and some registers to set the routing and enable
> the differnet (sic!) blocks of the display subsystem.
>
> Up to now both drivers, clock and drm are probed with the same device tree
> compatible. But only the first driver get probed, which in effect breaks
> graphics on mt8173 and mt2701.
Ouch!
>
> This patch uses a platform device registration in the DRM driver, which
> will trigger the probe of the corresponding clock driver. It was tested on the
> bananapi-r2 and the Acer R13 Chromebook."
Alright, please don't add nodes in DT just to make device drivers probe.
Instead, register clks from the drm driver or create a child platform
device for the clk bits purely in the drm driver and have that probe the
associated clk driver from there.
>
> DT is broken right now, because two drivers rely on the same node, which gets
> consumed just once. The new DT introduced does not break anything because it
> is
> only used for boards that: "[..] are not available to the general public
> (mt2712e) or only have the mmsys clock driver part implemented (mt6797)."
Ok, so backwards compatibility is irrelevant then. Sounds fine to me.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
On 21/11/2018 17:46, Stephen Boyd wrote:
> Quoting Rob Herring (2018-11-19 11:15:16)
>> On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
>> wrote:
>>> On 11/17/18 12:15 AM, Rob Herring wrote:
On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
> -#clock-cells = <1>;
> +
> +mmsys_clk: clock-controller@1400 {
> +compatible = "mediatek,mt2712-mmsys-clk";
> +#clock-cells = <1>;
This goes against the general direction of not defining separate nodes
for providers with no resources.
Why do you need this and what does it buy if you have to continue to
support the existing chips?
>>>
>>> It would show explicitly that the mmsys block is used to probe two
>>> drivers, one for the gpu and one for the clocks. Otherwise that is
>>> hidden in the drm driver code. I think it is cleaner to describe that in
>>> the device tree.
>>
>> No, that's maybe cleaner for the driver implementation in the Linux
>> kernel. What about other OS's or when Linux drivers and subsystems
>> needs change? Cleaner for DT is design bindings that reflect the h/w.
>> Hardware is sometimes just messy.
>>
>
> I agree. I fail to see what this patch series is doing besides changing
> driver probe and device creation methods and making a backwards
> incompatible change to DT. Is there any other benefit here?
>
You are referring whole series?
Citing the cover letter:
"MMSYS in Mediatek SoCs has some registers to control clock gates (which is
used in the clk driver) and some registers to set the routing and enable
the differnet (sic!) blocks of the display subsystem.
Up to now both drivers, clock and drm are probed with the same device tree
compatible. But only the first driver get probed, which in effect breaks
graphics on mt8173 and mt2701.
This patch uses a platform device registration in the DRM driver, which
will trigger the probe of the corresponding clock driver. It was tested on the
bananapi-r2 and the Acer R13 Chromebook."
DT is broken right now, because two drivers rely on the same node, which gets
consumed just once. The new DT introduced does not break anything because it is
only used for boards that: "[..] are not available to the general public
(mt2712e) or only have the mmsys clock driver part implemented (mt6797)."
Anyway, I'll send a new version which uses the platform device in the DRM driver
for all SoCs.
Regards,
Matthias
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
Quoting Rob Herring (2018-11-19 11:15:16)
> On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
> wrote:
> > On 11/17/18 12:15 AM, Rob Herring wrote:
> > > On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
> > >> -#clock-cells = <1>;
> > >> +
> > >> +mmsys_clk: clock-controller@1400 {
> > >> +compatible = "mediatek,mt2712-mmsys-clk";
> > >> +#clock-cells = <1>;
> > >
> > > This goes against the general direction of not defining separate nodes
> > > for providers with no resources.
> > >
> > > Why do you need this and what does it buy if you have to continue to
> > > support the existing chips?
> > >
> >
> > It would show explicitly that the mmsys block is used to probe two
> > drivers, one for the gpu and one for the clocks. Otherwise that is
> > hidden in the drm driver code. I think it is cleaner to describe that in
> > the device tree.
>
> No, that's maybe cleaner for the driver implementation in the Linux
> kernel. What about other OS's or when Linux drivers and subsystems
> needs change? Cleaner for DT is design bindings that reflect the h/w.
> Hardware is sometimes just messy.
>
I agree. I fail to see what this patch series is doing besides changing
driver probe and device creation methods and making a backwards
incompatible change to DT. Is there any other benefit here?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
On Sun, Nov 18, 2018 at 11:12 AM Matthias Brugger
wrote:
>
>
>
> On 11/17/18 12:15 AM, Rob Herring wrote:
> > On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
> >> From: Matthias Brugger
> >>
> >> On SoCs with no publical available HW or no working graphic stack
> >> we change the devicetree binding for the mmsys clock part. This
> >> way we don't need to register a platform device explicitly in the
> >> drm driver. Instead we can create a mmsys child which invokes the
> >> clock driver.
> >>
> >> Signed-off-by: Matthias Brugger
> >> ---
> >> .../bindings/arm/mediatek/mediatek,mmsys.txt | 21 ---
> >> .../display/mediatek/mediatek,disp.txt| 4
> >> 2 files changed, 18 insertions(+), 7 deletions(-)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >> index 4468345f8b1a..d4e205981363 100644
> >> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >> @@ -1,4 +1,4 @@
> >> -Mediatek mmsys controller
> >> +Mediatek mmsys clock controller
> >>
> >>
> >> The Mediatek mmsys controller provides various clocks to the system.
> >> @@ -6,18 +6,25 @@ The Mediatek mmsys controller provides various clocks to
> >> the system.
> >> Required Properties:
> >>
> >> - compatible: Should be one of:
> >> -- "mediatek,mt2712-mmsys", "syscon"
> >> -- "mediatek,mt6797-mmsys", "syscon"
> >> +- "mediatek,mt2712-mmsys-clk", "syscon"
> >> +- "mediatek,mt6797-mmsys-clk", "syscon"
> >
> > Doesn't match the example.>
> >> - #clock-cells: Must be 1
> >>
> >> -The mmsys controller uses the common clk binding from
> >> +The mmsys clock controller uses the common clk binding from
> >> Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> The available clocks are defined in dt-bindings/clock/mt*-clk.h.
> >> +It is a child of the mmsys block, see binding at:
> >> +Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
> >>
> >> Example:
> >>
> >> -mmsys: clock-controller@1400 {
> >> -compatible = "mediatek,mt8173-mmsys", "syscon";
> >> +mmsys: syscon@1400 {
> >> +compatible = "mediatek,mt2712-mmsys", "syscon", "simple-mfd";
> >> reg = <0 0x1400 0 0x1000>;
> >> -#clock-cells = <1>;
> >> +
> >> +mmsys_clk: clock-controller@1400 {
> >> +compatible = "mediatek,mt2712-mmsys-clk";
> >> +#clock-cells = <1>;
> >
> > This goes against the general direction of not defining separate nodes
> > for providers with no resources.
> >
> > Why do you need this and what does it buy if you have to continue to
> > support the existing chips?
> >
>
> It would show explicitly that the mmsys block is used to probe two
> drivers, one for the gpu and one for the clocks. Otherwise that is
> hidden in the drm driver code. I think it is cleaner to describe that in
> the device tree.
No, that's maybe cleaner for the driver implementation in the Linux
kernel. What about other OS's or when Linux drivers and subsystems
needs change? Cleaner for DT is design bindings that reflect the h/w.
Hardware is sometimes just messy.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
On 11/17/18 12:15 AM, Rob Herring wrote:
> On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
>> From: Matthias Brugger
>>
>> On SoCs with no publical available HW or no working graphic stack
>> we change the devicetree binding for the mmsys clock part. This
>> way we don't need to register a platform device explicitly in the
>> drm driver. Instead we can create a mmsys child which invokes the
>> clock driver.
>>
>> Signed-off-by: Matthias Brugger
>> ---
>> .../bindings/arm/mediatek/mediatek,mmsys.txt | 21 ---
>> .../display/mediatek/mediatek,disp.txt| 4
>> 2 files changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
>> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
>> index 4468345f8b1a..d4e205981363 100644
>> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
>> @@ -1,4 +1,4 @@
>> -Mediatek mmsys controller
>> +Mediatek mmsys clock controller
>>
>>
>> The Mediatek mmsys controller provides various clocks to the system.
>> @@ -6,18 +6,25 @@ The Mediatek mmsys controller provides various clocks to
>> the system.
>> Required Properties:
>>
>> - compatible: Should be one of:
>> -- "mediatek,mt2712-mmsys", "syscon"
>> -- "mediatek,mt6797-mmsys", "syscon"
>> +- "mediatek,mt2712-mmsys-clk", "syscon"
>> +- "mediatek,mt6797-mmsys-clk", "syscon"
>
> Doesn't match the example.>
>> - #clock-cells: Must be 1
>>
>> -The mmsys controller uses the common clk binding from
>> +The mmsys clock controller uses the common clk binding from
>> Documentation/devicetree/bindings/clock/clock-bindings.txt
>> The available clocks are defined in dt-bindings/clock/mt*-clk.h.
>> +It is a child of the mmsys block, see binding at:
>> +Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
>>
>> Example:
>>
>> -mmsys: clock-controller@1400 {
>> -compatible = "mediatek,mt8173-mmsys", "syscon";
>> +mmsys: syscon@1400 {
>> +compatible = "mediatek,mt2712-mmsys", "syscon", "simple-mfd";
>> reg = <0 0x1400 0 0x1000>;
>> -#clock-cells = <1>;
>> +
>> +mmsys_clk: clock-controller@1400 {
>> +compatible = "mediatek,mt2712-mmsys-clk";
>> +#clock-cells = <1>;
>
> This goes against the general direction of not defining separate nodes
> for providers with no resources.
>
> Why do you need this and what does it buy if you have to continue to
> support the existing chips?
>
It would show explicitly that the mmsys block is used to probe two
drivers, one for the gpu and one for the clocks. Otherwise that is
hidden in the drm driver code. I think it is cleaner to describe that in
the device tree.
pEpkey.asc
Description: application/pgp-keys
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias@kernel.org wrote:
> From: Matthias Brugger
>
> On SoCs with no publical available HW or no working graphic stack
> we change the devicetree binding for the mmsys clock part. This
> way we don't need to register a platform device explicitly in the
> drm driver. Instead we can create a mmsys child which invokes the
> clock driver.
>
> Signed-off-by: Matthias Brugger
> ---
> .../bindings/arm/mediatek/mediatek,mmsys.txt | 21 ---
> .../display/mediatek/mediatek,disp.txt| 4
> 2 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> index 4468345f8b1a..d4e205981363 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> @@ -1,4 +1,4 @@
> -Mediatek mmsys controller
> +Mediatek mmsys clock controller
>
>
> The Mediatek mmsys controller provides various clocks to the system.
> @@ -6,18 +6,25 @@ The Mediatek mmsys controller provides various clocks to
> the system.
> Required Properties:
>
> - compatible: Should be one of:
> - - "mediatek,mt2712-mmsys", "syscon"
> - - "mediatek,mt6797-mmsys", "syscon"
> + - "mediatek,mt2712-mmsys-clk", "syscon"
> + - "mediatek,mt6797-mmsys-clk", "syscon"
Doesn't match the example.
> - #clock-cells: Must be 1
>
> -The mmsys controller uses the common clk binding from
> +The mmsys clock controller uses the common clk binding from
> Documentation/devicetree/bindings/clock/clock-bindings.txt
> The available clocks are defined in dt-bindings/clock/mt*-clk.h.
> +It is a child of the mmsys block, see binding at:
> +Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
>
> Example:
>
> -mmsys: clock-controller@1400 {
> - compatible = "mediatek,mt8173-mmsys", "syscon";
> +mmsys: syscon@1400 {
> + compatible = "mediatek,mt2712-mmsys", "syscon", "simple-mfd";
> reg = <0 0x1400 0 0x1000>;
> - #clock-cells = <1>;
> +
> + mmsys_clk: clock-controller@1400 {
> + compatible = "mediatek,mt2712-mmsys-clk";
> + #clock-cells = <1>;
This goes against the general direction of not defining separate nodes
for providers with no resources.
Why do you need this and what does it buy if you have to continue to
support the existing chips?
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 08/12] dt-bindings: mediatek: Change the binding for mmsys clocks
From: Matthias Brugger
On SoCs with no publical available HW or no working graphic stack
we change the devicetree binding for the mmsys clock part. This
way we don't need to register a platform device explicitly in the
drm driver. Instead we can create a mmsys child which invokes the
clock driver.
Signed-off-by: Matthias Brugger
---
.../bindings/arm/mediatek/mediatek,mmsys.txt | 21 ---
.../display/mediatek/mediatek,disp.txt| 4
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
index 4468345f8b1a..d4e205981363 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
@@ -1,4 +1,4 @@
-Mediatek mmsys controller
+Mediatek mmsys clock controller
The Mediatek mmsys controller provides various clocks to the system.
@@ -6,18 +6,25 @@ The Mediatek mmsys controller provides various clocks to the
system.
Required Properties:
- compatible: Should be one of:
- - "mediatek,mt2712-mmsys", "syscon"
- - "mediatek,mt6797-mmsys", "syscon"
+ - "mediatek,mt2712-mmsys-clk", "syscon"
+ - "mediatek,mt6797-mmsys-clk", "syscon"
- #clock-cells: Must be 1
-The mmsys controller uses the common clk binding from
+The mmsys clock controller uses the common clk binding from
Documentation/devicetree/bindings/clock/clock-bindings.txt
The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+It is a child of the mmsys block, see binding at:
+Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
Example:
-mmsys: clock-controller@1400 {
- compatible = "mediatek,mt8173-mmsys", "syscon";
+mmsys: syscon@1400 {
+ compatible = "mediatek,mt2712-mmsys", "syscon", "simple-mfd";
reg = <0 0x1400 0 0x1000>;
- #clock-cells = <1>;
+
+ mmsys_clk: clock-controller@1400 {
+ compatible = "mediatek,mt2712-mmsys-clk";
+ #clock-cells = <1>;
+ };
+
};
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
index 4b008d992398..38c708cb7e55 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
@@ -54,6 +54,10 @@ Required properties (all function blocks):
DPI controller nodes have multiple clock inputs. These are documented in
mediatek,dsi.txt and mediatek,dpi.txt, respectively.
+Some chips have a separate binding for the clock controller, which is a child
node
+of the mmsys device, for more information see:
+Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
+
Required properties (DMA function blocks):
- compatible: Should be one of
"mediatek,-disp-ovl"
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
