From: Rahul Sharma <rahul.sharma@samsung.com>
To: Tomasz Figa <t.figa@samsung.com>
Cc: Tomasz Stanislawski <t.stanislaws@samsung.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
Kishon Vijay Abraham I <kishon@ti.com>,
PANKAJ KUMAR DUBEY <pankaj.dubey@samsung.com>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Andrzej Hajda <a.hajda@samsung.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Rob Herring <robh+dt@kernel.org>,
Grant Likely <grant.likely@linaro.org>,
Kukjin Kim <kgene.kim@samsung.com>,
Sylwester Nawrocki <sylvester.nawrocki@gmail.com>,
sunil joshi <joshi@samsung.com>
Subject: Re: [PATCH v3 1/3] phy: Add exynos-simple-phy driver
Date: Mon, 19 May 2014 12:40:59 +0530 [thread overview]
Message-ID: <CAPdUM4OWbDBDNsaETT64sbLs_We40ZeV2WVh09Jkdx_ZPoms4Q@mail.gmail.com> (raw)
In-Reply-To: <53762583.5020907@samsung.com>
On 16 May 2014 20:19, Tomasz Figa <t.figa@samsung.com> wrote:
> On 16.05.2014 16:30, Rahul Sharma wrote:
>> On 16 May 2014 16:20, Tomasz Figa <t.figa@samsung.com> wrote:
>>> On 16.05.2014 12:35, Rahul Sharma wrote:
>>>> On 16 May 2014 15:12, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>>>>> On 16 May 2014 03:14, Tomasz Figa <tomasz.figa@gmail.com> 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.
@Tomasz Stanislawski, Do you have different opinion here?
Regards,
Rahul Sharma.
>
> Best regards,
> Tomasz
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2014-05-19 7:11 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-14 19:17 [PATCH v3 0/3] phy: Add exynos-simple-phy driver Rahul Sharma
2014-05-14 19:17 ` [PATCH v3 1/3] " Rahul Sharma
2014-05-14 20:01 ` Tomasz Figa
2014-05-15 4:01 ` Rahul Sharma
2014-05-15 21:44 ` Tomasz Figa
2014-05-16 9:42 ` Rahul Sharma
2014-05-16 10:35 ` Rahul Sharma
2014-05-16 10:50 ` Tomasz Figa
2014-05-16 14:30 ` Rahul Sharma
2014-05-16 14:49 ` Tomasz Figa
2014-05-19 7:10 ` Rahul Sharma [this message]
2014-05-19 10:54 ` Tomasz Figa
2014-05-20 5:12 ` Rahul Sharma
2014-05-14 22:14 ` Thierry Reding
2014-05-15 5:19 ` Rahul Sharma
2014-05-15 7:42 ` Thierry Reding
2014-05-15 8:17 ` Rahul Sharma
2014-05-15 9:23 ` Thierry Reding
2014-05-15 13:31 ` Bartlomiej Zolnierkiewicz
2014-05-15 13:35 ` Rahul Sharma
2014-05-15 13:41 ` Kishon Vijay Abraham I
2014-05-15 13:45 ` Rahul Sharma
2014-05-14 19:17 ` [PATCH v3 2/3] drm: exynos: hdmi: use hdmiphy as PHY Rahul Sharma
2014-05-14 19:17 ` [PATCH v3 3/3] s5p-tv: " Rahul Sharma
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=CAPdUM4OWbDBDNsaETT64sbLs_We40ZeV2WVh09Jkdx_ZPoms4Q@mail.gmail.com \
--to=rahul.sharma@samsung.com \
--cc=a.hajda@samsung.com \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=grant.likely@linaro.org \
--cc=joshi@samsung.com \
--cc=kgene.kim@samsung.com \
--cc=kishon@ti.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=pankaj.dubey@samsung.com \
--cc=robh+dt@kernel.org \
--cc=sylvester.nawrocki@gmail.com \
--cc=t.figa@samsung.com \
--cc=t.stanislaws@samsung.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).