devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Neil Armstrong <narmstrong@baylibre.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Viktor Prutyanov <viktor.prutyanov@phystech.edu>
Cc: sean@mess.org, mchehab@kernel.org, robh+dt@kernel.org,
	khilman@baylibre.com, jbrunet@baylibre.com,
	linux-media <linux-media@vger.kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-amlogic@lists.infradead.org, rockosov@gmail.com
Subject: Re: [PATCH 1/2] media: rc: meson-irblaster: document device tree bindings
Date: Fri, 2 Jul 2021 15:48:39 +0200	[thread overview]
Message-ID: <cc677601-fb6b-970d-be8a-e68bea9727dd@baylibre.com> (raw)
In-Reply-To: <CAFBinCBZXRrPakwvRLsbNbuYY6fcYysMs0+SPUmAhKpcYxrq0g@mail.gmail.com>

Hi,

On 02/07/2021 15:30, Martin Blumenstingl wrote:
> Hi Viktor,
> 
> On Thu, Jul 1, 2021 at 11:51 PM Viktor Prutyanov
> <viktor.prutyanov@phystech.edu> wrote:
>>
>> This patch adds binding documentation for the IR transmitter
>> available in Amlogic Meson SoCs.
> This is an interesting piece of hardware where I've always wondered if
> there is any device out there which supports this functionality.It
> turns out that there is

You did beat me, I started a driver some time ago but failed to finish debugging it...
https://github.com/superna9999/linux/tree/amlogic/v5.2%2Fir-blaster

> 
> [...]
>> +description: |
>> +  Some Amlogic SoCs such as A311D and T950D4 have IR transmitter
>> +  (blaster) controller onboard. It is capable of sending IR signals
>> +  with arbitrary carrier frequency and duty cycle.
>> +
>> +properties:
>> +  compatible:
>> +    const: amlogic,meson-irblaster
> if you feel like some registers or register values are specific to
> A311D or T950D4 then please also add a SoC-specific compatible string
> (for example: amlogic,meson-g12b-irblaster).
> An example can be seen in
> Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yaml

AFAIK there is 2 versions of the IP, the "old" one we can find on Meson6, 8/8b, GXBB, GXL & GXM (and maybe AXG ?),
and the one we find on the latest G12A, G12B & SM1.

The SEI510 and SEI610 boards we use for Yukawa android port do have the necessary HW for IR sending,
so I'll eventually be able to test.

So, as martin says you should add a "amlogic,g12a-ir-blaster" to be sure we support the older ir blaster version
correctly with the right bindings.

Neil

> 
> [...]
>> +  clocks:
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    minItems: 1
>> +    maxItems: 2
> from my understanding there are two clock inputs to the hardware
> dt-bindings should always describe the hardware, not what the driver
> may (or may not) use.
> based on that I think you should drop minItems (then minItems will
> have the same value as maxItems)
> 
> [...]
>> +  mod-clock:
>> +    oneOf:
>> +      - const: sysclk
>> +      - const: xtal
> Does this "mod-clock" depend on something external to the IR blaster hardware?
> If not this should be handled inside the driver only.
> 
> From how I understand the register description in the datasheet
> there's two clock inputs.
> XTAL is internally divided further down with fixed dividers.
> Then there's a configurable divider which is then used to generate the
> IR signal.
> If the sysclk (I assume that this is clk81 - or at least derived from
> it) is "too fast" then the driver should just ignore that clock while
> the dt-bindings should still describe it (see my comment above)
> 
> [...]
>> +    meson-irblaster@ff80014c {
> node names should be generic, see for example
> Documentation/devicetree/bindings/spi/amlogic,meson6-spifc.yaml
> (spifc is the name Amlogic has given this IP, but since node names are
> supposed to be generic we use spi@...)
> 
> However, I am not sure if an IR blaster would be described as
> ir-blaster@... or simply ir@...
> 
>> +      compatible = "amlogic,meson-irblaster";
>> +      reg = <0xff80014c 0x10>;
>> +      interrupts = <0 198 IRQ_TYPE_EDGE_RISING>;
>> +      clocks = <&clkc CLKID_CLK81 &xtal>;
> [...]
>> +      clocks = <&clkc CLKID_CLK81 &xtal>;
> while this works I think the recommended format is:
>     clocks = <&clkc CLKID_CLK81>, <&xtal>
> 
> 
> Best regards,
> Martin
> 

  reply	other threads:[~2021-07-02 13:48 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 21:51 [PATCH 0/2] media: rc: add support for Amlogic Meson IR blaster Viktor Prutyanov
2021-07-01 21:51 ` [PATCH 1/2] media: rc: meson-irblaster: document device tree bindings Viktor Prutyanov
2021-07-02 13:30   ` Martin Blumenstingl
2021-07-02 13:48     ` Neil Armstrong [this message]
2021-07-01 21:51 ` [PATCH 2/2] media: rc: introduce Meson IR blaster driver Viktor Prutyanov
2021-07-01 22:46   ` Sean Young
2021-07-02 16:15     ` Martin Blumenstingl
2021-07-02 20:23       ` Viktor Prutyanov
2021-07-07 14:40     ` Viktor Prutyanov
2021-07-02  1:24   ` kernel test robot
2021-07-02 22:39   ` kernel test robot

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=cc677601-fb6b-970d-be8a-e68bea9727dd@baylibre.com \
    --to=narmstrong@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=mchehab@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=rockosov@gmail.com \
    --cc=sean@mess.org \
    --cc=viktor.prutyanov@phystech.edu \
    /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).