| Message ID | 1483632384-8107-2-git-send-email-sean.wang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 06, 2017 at 12:06:23AM +0800, sean.wang@mediatek.com wrote: > From: Sean Wang <sean.wang@mediatek.com> > > This patch adds documentation for devicetree bindings for > Mediatek IR controller. > > Signed-off-by: Sean Wang <sean.wang@mediatek.com> > --- > .../devicetree/bindings/media/mtk-cir.txt | 23 ++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > create mode 100644 linux-4.8.rc1_p0/Documentation/devicetree/bindings/media/mtk-cir.txt > > diff --git a/Documentation/devicetree/bindings/media/mtk-cir.txt b/Documentation/devicetree/bindings/media/mtk-cir.txt > new file mode 100644 > index 0000000..bbedd71 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/mtk-cir.txt > @@ -0,0 +1,23 @@ > +Device-Tree bindings for Mediatek IR controller found in Mediatek SoC family > + > +Required properties: > +- compatible : "mediatek,mt7623-ir" > +- clocks : list of clock specifiers, corresponding to > + entries in clock-names property; > +- clock-names : should contain "clk" entries; > +- interrupts : should contain IR IRQ number; > +- reg : should contain IO map address for IR. > + > +Optional properties: > +- linux,rc-map-name : Remote control map name. Would 'label' be appropriate here instead? If not, this needs to be documented in a common location and explained better. > + > +Example: > + > +cir: cir@0x10013000 { Drop the '0x'. > + compatible = "mediatek,mt7623-ir"; > + reg = <0 0x10013000 0 0x1000>; > + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&infracfg CLK_INFRA_IRRX>; > + clock-names = "clk"; > + linux,rc-map-name = "rc-rc6-mce"; > +}; > -- > 1.9.1 >
Hi Rob, thanks for your effort for reviewing. I added comments inline. On Mon, 2017-01-09 at 12:32 -0600, Rob Herring wrote: > On Fri, Jan 06, 2017 at 12:06:23AM +0800, sean.wang@mediatek.com wrote: > > From: Sean Wang <sean.wang@mediatek.com> > > > > This patch adds documentation for devicetree bindings for > > Mediatek IR controller. > > > > Signed-off-by: Sean Wang <sean.wang@mediatek.com> > > --- > > .../devicetree/bindings/media/mtk-cir.txt | 23 ++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > create mode 100644 linux-4.8.rc1_p0/Documentation/devicetree/bindings/media/mtk-cir.txt > > > > diff --git a/Documentation/devicetree/bindings/media/mtk-cir.txt b/Documentation/devicetree/bindings/media/mtk-cir.txt > > new file mode 100644 > > index 0000000..bbedd71 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/mtk-cir.txt > > @@ -0,0 +1,23 @@ > > +Device-Tree bindings for Mediatek IR controller found in Mediatek SoC family > > + > > +Required properties: > > +- compatible : "mediatek,mt7623-ir" > > +- clocks : list of clock specifiers, corresponding to > > + entries in clock-names property; > > +- clock-names : should contain "clk" entries; > > +- interrupts : should contain IR IRQ number; > > +- reg : should contain IO map address for IR. > > + > > +Optional properties: > > +- linux,rc-map-name : Remote control map name. > > Would 'label' be appropriate here instead? If not, this needs to be > documented in a common location and explained better. > I checked with how the way applied in other IR drivers is and found that most IR driver also use the same label to identify the scan/key table they prefer to use such as gpio-ir-recv, ir-hix5hd2, meson-ir and sunxi-cir or use hard coding inside the driver. So I thought it should be appropriate here currently. > > + > > +Example: > > + > > +cir: cir@0x10013000 { > > Drop the '0x'. > okay, I will. > > + compatible = "mediatek,mt7623-ir"; > > + reg = <0 0x10013000 0 0x1000>; > > + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_LOW>; > > + clocks = <&infracfg CLK_INFRA_IRRX>; > > + clock-names = "clk"; > > + linux,rc-map-name = "rc-rc6-mce"; > > +}; > > -- > > 1.9.1 > >
On Mon, Jan 9, 2017 at 8:35 PM, Sean Wang <sean.wang@mediatek.com> wrote: > Hi Rob, > > thanks for your effort for reviewing. I added comments inline. > > On Mon, 2017-01-09 at 12:32 -0600, Rob Herring wrote: >> On Fri, Jan 06, 2017 at 12:06:23AM +0800, sean.wang@mediatek.com wrote: >> > From: Sean Wang <sean.wang@mediatek.com> >> > >> > This patch adds documentation for devicetree bindings for >> > Mediatek IR controller. >> > >> > Signed-off-by: Sean Wang <sean.wang@mediatek.com> >> > --- >> > .../devicetree/bindings/media/mtk-cir.txt | 23 ++++++++++++++++++++++ >> > 1 file changed, 23 insertions(+) >> > create mode 100644 linux-4.8.rc1_p0/Documentation/devicetree/bindings/media/mtk-cir.txt >> > >> > diff --git a/Documentation/devicetree/bindings/media/mtk-cir.txt b/Documentation/devicetree/bindings/media/mtk-cir.txt >> > new file mode 100644 >> > index 0000000..bbedd71 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/media/mtk-cir.txt >> > @@ -0,0 +1,23 @@ >> > +Device-Tree bindings for Mediatek IR controller found in Mediatek SoC family >> > + >> > +Required properties: >> > +- compatible : "mediatek,mt7623-ir" >> > +- clocks : list of clock specifiers, corresponding to >> > + entries in clock-names property; >> > +- clock-names : should contain "clk" entries; >> > +- interrupts : should contain IR IRQ number; >> > +- reg : should contain IO map address for IR. >> > + >> > +Optional properties: >> > +- linux,rc-map-name : Remote control map name. >> >> Would 'label' be appropriate here instead? If not, this needs to be >> documented in a common location and explained better. >> > I checked with how the way applied in other IR drivers is and found that > most IR driver also use the same label to identify the scan/key table > they prefer to use such as gpio-ir-recv, ir-hix5hd2, meson-ir and > sunxi-cir or use hard coding inside the driver. So I thought it should > be appropriate here currently. Maybe so, but anything with linux prefix gets extra scrutiny and I'm not sure that happened on the previous cases. If label has the same meaning, then we should start using that and deprecate this property. In any case, a property used by multiple bindings needs to be documented in a common place. The explanation of the property is bad too. It just spells out RC with no explanation. I'm sure you just copy-n-pasted it from the others, but that doesn't make it okay. Rob
diff --git a/Documentation/devicetree/bindings/media/mtk-cir.txt b/Documentation/devicetree/bindings/media/mtk-cir.txt new file mode 100644 index 0000000..bbedd71 --- /dev/null +++ b/Documentation/devicetree/bindings/media/mtk-cir.txt @@ -0,0 +1,23 @@ +Device-Tree bindings for Mediatek IR controller found in Mediatek SoC family + +Required properties: +- compatible : "mediatek,mt7623-ir" +- clocks : list of clock specifiers, corresponding to + entries in clock-names property; +- clock-names : should contain "clk" entries; +- interrupts : should contain IR IRQ number; +- reg : should contain IO map address for IR. + +Optional properties: +- linux,rc-map-name : Remote control map name. + +Example: + +cir: cir@0x10013000 { + compatible = "mediatek,mt7623-ir"; + reg = <0 0x10013000 0 0x1000>; + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_LOW>; + clocks = <&infracfg CLK_INFRA_IRRX>; + clock-names = "clk"; + linux,rc-map-name = "rc-rc6-mce"; +};