From: <Dharma.B@microchip.com>
To: <conor@kernel.org>
Cc: <Conor.Dooley@microchip.com>, <sam@ravnborg.org>,
<bbrezillon@kernel.org>, <maarten.lankhorst@linux.intel.com>,
<mripard@kernel.org>, <tzimmermann@suse.de>, <airlied@gmail.com>,
<daniel@ffwll.ch>, <robh+dt@kernel.org>,
<krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
<Nicolas.Ferre@microchip.com>, <alexandre.belloni@bootlin.com>,
<claudiu.beznea@tuxon.dev>, <dri-devel@lists.freedesktop.org>,
<devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <lee@kernel.org>,
<thierry.reding@gmail.com>, <u.kleine-koenig@pengutronix.de>,
<linux-pwm@vger.kernel.org>, <Linux4Microchip@microchip.com>
Subject: Re: [PATCH v3 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format
Date: Wed, 24 Jan 2024 05:18:26 +0000 [thread overview]
Message-ID: <4906b7e2-0ddb-4d3c-a48b-e16278f2d649@microchip.com> (raw)
In-Reply-To: <20240122-stark-duress-2f59294dcf27@spud>
Hi Conor & All,
On 22/01/24 2:46 pm, Conor Dooley wrote:
> On Mon, Jan 22, 2024 at 03:38:41AM +0000,Dharma.B@microchip.com wrote:
>> Hi Conor,
>> On 19/01/24 5:33 pm, Conor Dooley - M52691 wrote:
>>> On Fri, Jan 19, 2024 at 03:32:49AM +0000,Dharma.B@microchip.com wrote:
>>>> On 18/01/24 9:10 pm, Conor Dooley wrote:
>>>>> On Thu, Jan 18, 2024 at 02:56:12PM +0530, Dharma Balasubiramani wrote:
>>>>>> Convert the atmel,hlcdc binding to DT schema format.
>>>>>>
>>>>>> Adjust the clock-names property to clarify that the LCD controller expects
>>>>>> one of these clocks (either sys_clk or lvds_pll_clk to be present but not
>>>>>> both) along with the slow_clk and periph_clk. This alignment with the actual
>>>>>> hardware requirements will enable accurate device tree configuration for
>>>>>> systems using the HLCDC IP.
>>>>>>
>>>>>> Signed-off-by: Dharma Balasubiramani<dharma.b@microchip.com>
>>>>>> ---
>>>>>> changelog
>>>>>> v2 -> v3
>>>>>> - Rename hlcdc-display-controller and hlcdc-pwm to generic names.
>>>>>> - Modify the description by removing the unwanted comments and '|'.
>>>>>> - Modify clock-names simpler.
>>>>>> v1 -> v2
>>>>>> - Remove the explicit copyrights.
>>>>>> - Modify title (not include words like binding/driver).
>>>>>> - Modify description actually describing the hardware and not the driver.
>>>>>> - Add details of lvds_pll addition in commit message.
>>>>>> - Ref endpoint and not endpoint-base.
>>>>>> - Fix coding style.
>>>>>> ...
>>>>>> .../devicetree/bindings/mfd/atmel,hlcdc.yaml | 97 +++++++++++++++++++
>>>>>> .../devicetree/bindings/mfd/atmel-hlcdc.txt | 56 -----------
>>>>>> 2 files changed, 97 insertions(+), 56 deletions(-)
>>>>>> create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>>>> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..eccc998ac42c
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>>>>>> @@ -0,0 +1,97 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id:http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
>>>>>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Atmel's HLCD Controller
>>>>>> +
>>>>>> +maintainers:
>>>>>> + - Nicolas Ferre<nicolas.ferre@microchip.com>
>>>>>> + - Alexandre Belloni<alexandre.belloni@bootlin.com>
>>>>>> + - Claudiu Beznea<claudiu.beznea@tuxon.dev>
>>>>>> +
>>>>>> +description:
>>>>>> + The Atmel HLCDC (HLCD Controller) IP available on Atmel SoCs exposes two
>>>>>> + subdevices, a PWM chip and a Display Controller.
>>>>>> +
>>>>>> +properties:
>>>>>> + compatible:
>>>>>> + enum:
>>>>>> + - atmel,at91sam9n12-hlcdc
>>>>>> + - atmel,at91sam9x5-hlcdc
>>>>>> + - atmel,sama5d2-hlcdc
>>>>>> + - atmel,sama5d3-hlcdc
>>>>>> + - atmel,sama5d4-hlcdc
>>>>>> + - microchip,sam9x60-hlcdc
>>>>>> + - microchip,sam9x75-xlcdc
>>>>>> +
>>>>>> + reg:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + interrupts:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + clocks:
>>>>>> + maxItems: 3
>>>>> Hmm, one thing I probably should have said on the previous version, but
>>>>> I missed somehow: It would be good to add an items list to the clocks
>>>>> property here to explain what the 3 clocks are/are used for - especially
>>>>> since there is additional complexity being added here to use either the
>>>>> sys or lvds clocks.
>>>> May I inquire if this approach is likely to be effective?
>>>>
>>>> clocks:
>>>> items:
>>>> - description: peripheral clock
>>>> - description: generic clock or lvds pll clock
>>>> Once the LVDS PLL is enabled, the pixel clock is used as the
>>>> clock for LCDC, so its GCLK is no longer needed.
>>>> - description: slow clock
>>>> maxItems: 3
>>> Hmm that sounds very suspect to me. "Once the lvdspll is enabled the
>>> generic clock is no longer needed" sounds like both clocks can be provided
>>> to the IP on different pins and their provision is not mutually
>>> exclusive, just that the IP will only actually use one at a time. If
>>> that is the case, then this patch is nott correct and the binding should
>>> allow for 4 clocks, with both the generic clock and the lvds pll being
>>> present in the DT at the same time.
>>>
>>> I vaguely recall internal discussion about this problem some time back
>>> but the details all escape me.
>> Let's delve deeper into the clock configuration for LCDC_PCK.
>>
>> Considering the flexibility of the design, it appears that both clocks,
>> sys_clk (generic clock) and lvds_pll_clk, can indeed be provided to the
>> IP simultaneously. The crucial aspect, however, is that the IP will
>> utilize only one of these clocks at any given time. This aligns with the
>> specific requirements of the application, where the choice of clock
>> depends on whether the LVDS interface or MIPI/DSI is in use.
> If both clocks can physically be provided to the IP then both of them
> should be in the dt. The hcldc appears to me to be a part of the SoC and
> the clock routing to the IP is likely fixed.
>
>> To ensure proper configuration of the pixel clock period, we need to
>> distinctly identify which clocks are being utilized. For instance, in
>> the LVDS interface scenario, the lvds_pll_clk is essential, resulting in
>> LCDC_PCK being set to the source clock. Conversely, in the MIPI/DSI
>> case, the LCDC GCLK is required, leading to LCDC_PCK being defined as
>> source clock/CLKDIV+2.
>>
>> Considering the potential coexistence of sys_clk and lvds_pll_clk in the
>> Device Tree (DT), we may need to introduce an additional flag in the DT.
>> This flag could serve as a clear indicator of whether the LVDS interface
>> or MIPI/DSI is being employed. As we discussed to drop this flag and
>> just have any one of the clocks I believe that this approach provides a
>> sensible and scalable solution, allowing for a comprehensive
>> representation of the clocking configuration.
> This is probably a question for the folks on the DRM or media side of
> things, but is it not possible to determine based on the endpoint what
> protocol is required?
> I know that on the media side of things there's an endpoint property
> that can be used to specific the bus-type - is there an equivalent
> property for DRM stuff?
Yes, it can be done.
I will have the lvds pll in the lvds DT node.
I will just convert the existing text binding to yaml without this
additonal lvds pll clock.
--
Thanks,
Dharma B.
>
> Cheers,
> Conor.
next prev parent reply other threads:[~2024-01-24 5:18 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-18 9:26 [PATCH v3 0/3] Convert Microchip's HLCDC Text based DT bindings to JSON schema Dharma Balasubiramani
2024-01-18 9:26 ` [PATCH v3 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema Dharma Balasubiramani
2024-01-18 15:31 ` Conor Dooley
2024-01-19 2:51 ` Dharma.B
2024-01-18 9:26 ` [PATCH v3 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema Dharma Balasubiramani
2024-01-18 10:16 ` Uwe Kleine-König
2024-01-20 10:03 ` Uwe Kleine-König
2024-01-19 19:45 ` Rob Herring
2024-01-24 9:58 ` Dharma.B
2024-01-18 9:26 ` [PATCH v3 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format Dharma Balasubiramani
2024-01-18 15:40 ` Conor Dooley
2024-01-19 3:32 ` Dharma.B
2024-01-19 12:03 ` Conor Dooley
2024-01-22 3:38 ` Dharma.B
2024-01-22 9:16 ` Conor Dooley
2024-01-24 5:18 ` Dharma.B [this message]
2024-01-24 16:39 ` Conor Dooley
2024-01-25 6:47 ` Dharma.B
2024-01-25 8:27 ` Conor Dooley
2024-01-26 14:22 ` Dharma.B
2024-01-26 15:33 ` Conor Dooley
2024-01-29 3:41 ` Dharma.B
2024-01-29 17:14 ` Conor Dooley
2024-01-18 19:30 ` [PATCH v3 0/3] Convert Microchip's HLCDC Text based DT bindings to JSON schema Sam Ravnborg
2024-01-19 8:41 ` Dharma.B
2024-01-19 21:33 ` Sam Ravnborg
2024-01-19 19:51 ` Rob Herring
2024-01-20 13:23 ` Sam Ravnborg
2024-01-22 3:52 ` Dharma.B
2024-01-22 16:02 ` Sam Ravnborg
2024-01-22 16:04 ` Sam Ravnborg
2024-01-24 8:55 ` Dharma.B
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=4906b7e2-0ddb-4d3c-a48b-e16278f2d649@microchip.com \
--to=dharma.b@microchip.com \
--cc=Conor.Dooley@microchip.com \
--cc=Linux4Microchip@microchip.com \
--cc=Nicolas.Ferre@microchip.com \
--cc=airlied@gmail.com \
--cc=alexandre.belloni@bootlin.com \
--cc=bbrezillon@kernel.org \
--cc=claudiu.beznea@tuxon.dev \
--cc=conor+dt@kernel.org \
--cc=conor@kernel.org \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=robh+dt@kernel.org \
--cc=sam@ravnborg.org \
--cc=thierry.reding@gmail.com \
--cc=tzimmermann@suse.de \
--cc=u.kleine-koenig@pengutronix.de \
/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).