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 >>>>>> <matthias.bgg@xxxxxxxxx> wrote: >>>>>>> On 11/17/18 12:15 AM, Rob Herring wrote: >>>>>>>> On Fri, Nov 16, 2018 at 01:54:45PM +0100, matthias.bgg@xxxxxxxxxx wrote: >>>>>>>>> - #clock-cells = <1>; >>>>>>>>> + >>>>>>>>> + mmsys_clk: clock-controller@14000000 { >>>>>>>>> + 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-mediatek@xxxxxxxxxxxxxxxxxxx >> http://lists.infradead.org/mailman/listinfo/linux-mediatek > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel