On 19 May 2014 16:24, Tomasz Figa <tomasz.figa@xxxxxxxxx> wrote: > On 19.05.2014 09:10, Rahul Sharma wrote: >> On 16 May 2014 20:19, Tomasz Figa <t.figa@xxxxxxxxxxx> wrote: >>> On 16.05.2014 16:30, Rahul Sharma wrote: >>>> On 16 May 2014 16:20, Tomasz Figa <t.figa@xxxxxxxxxxx> wrote: >>>>> On 16.05.2014 12:35, Rahul Sharma wrote: >>>>>> On 16 May 2014 15:12, Rahul Sharma <rahul.sharma@xxxxxxxxxxx> wrote: >>>>>>> On 16 May 2014 03:14, Tomasz Figa <tomasz.figa@xxxxxxxxx> wrote: >>>>>>>> On 15.05.2014 06:01, Rahul Sharma wrote: >>>>>> [snip] >>>>>>>>>> the PHY provider. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Please correct me if I got you wrong. You want somthing like this: >>>>>>>>> >>>>>>>>> pmu_system_controller: system-controller@10040000 { >>>>>>>>> ... >>>>>>>>> simple_phys: simple-phys { >>>>>>>>> compatible = "samsung,exynos5420-simple-phy"; >>>>>>>>> ... >>>>>>>>> }; >>>>>>>>> }; >>>>>>>> >>>>>>>> Not exactly. >>>>>>>> >>>>>>>> What I meant is that the PMU node itself should be the PHY provider, e.g. >>>>>>>> >>>>>>>> pmu_system_controller: system-controller@10040000 { >>>>>>>> /* ... */ >>>>>>>> #phy-cells = <1>; >>>>>>>> }; >>>>>>>> >>>>>>>> and then the PMU node should instantiate the Exynos simple PHY driver, >>>>>>>> as this is a driver for a facility existing entirely inside of the PMU. >>>>>>>> Moreover, the driver should be rather called Exynos PMU PHY. >>>>>>>> >>>>>>>> I know this isn't really possible at the moment, but with device tree we >>>>>>>> must design things carefully, so it's better to take a bit more time and >>>>>>>> do things properly. >>>>>>>> >>>>>>>> So my opinion on this is that there should be a central Exynos PMU >>>>>>>> driver that claims the IO region and instantiates necessary subdrivers, >>>>>>>> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle >>>>>>>> driver and more, similar to what is being done in drivers/mfd. >>>>>>> >>>>>> >>>>>> Hi Tomasz, >>>>>> >>>>>> These PHYs are not part of PMU as such. I am not sure if it is correct to >>>>>> probe them as phy provider for all these phys. Only relation of these phys with >>>>>> the PMU is 'enable/disable control'. >>>>> >>>>> Well, in reality what is implemented by this driver is not even a PHY, >>>>> just some kind of power controllers, which are contained entirely in the >>>>> PMU. >>>>> >>>> >>>> I agree. Actually the role of generic phy framework for these 'simple' phys is >>>> only that much. >>>> >>>>>> Controlling this bit using regmap interface >>>>>> still looks better to me. >>>>> >>>>> Well, when there is a choice between using regmap and not using regmap, >>>>> I'd rather choose the latter. Why would you want to introduce additional >>>>> abstraction layer if there is no need for such? >>>>> >>>>>> >>>>>> IMHO Ideal method would be probing these PHYs independently and resolving >>>>>> the necessary dependencies like syscon handle, clocks etc. This way we will >>>>>> not be having any common phy provider for all these independent PHYs and it >>>>>> would be clean to add each of these phy nodes in DT. Please see my original >>>>>> comment below. >>>>>> >>>>>> http://lkml.iu.edu/hypermail/linux/kernel/1404.1/00701.html >>>>> >>>>> With the solution I proposed, you don't need any kind of dependencies >>>>> for those simple power controllers. They are just single bits that don't >>>>> need anything special to operate, except PMU clock running. >>>> >>>> In that case we can further trim it down and let the drivers use the regmap >>>> interface to control this bit. Many drivers including HDMI, DP just need that >>>> much functionality from the phy provider. >>> >>> Well, this is what several drivers already do, like USB PHY (dedicated >>> IP block), watchdog (for watchdog mask), SATA PHY (dedicated IP block >>> too) or will do, like I2C (for configuration of I2C mux on Exynos5). >>> >>> At least this would be consistent with them and wouldn't be an API >>> abuse, so I'd be inclined to go this way more than introducing >>> abstractions like this patch does. >> >> Ok. I had already posted a patch for this at >> http://www.spinics.net/lists/linux-samsung-soc/msg28049.html >> I will revive that thread. > > Looks good to me. > >> >> @Tomasz Stanislawski, Do you have different opinion here? > > I'm afraid Tomasz might not be very responsive during next few days, as > he is on a business trip. You might be able to reach him on our internal > communicator, though. Thanks Tomasz, I will contact him over communicator. Regards. > > Best regards, > Tomasz > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html