Message ID | 20171217224547.21481-1-embed3d@gmail.com |
---|---|
Headers | show |
Series | arm: sunxi: IR support for A83T | expand |
On Sun, Dec 17, 2017 at 11:45:43PM +0100, Philipp Rossak wrote: > This patch updates the sunxi-ir driver to set the base clock frequency from > devicetree. > > This is neccessary since there are different ir recievers on the > market, that operate with different frequencys. So this value could be s/neccessary/necessary/ s/recievers/receivers/ s/frequencys/frequencies/ > set if the attached ir receiver needs an other base clock frequency, > than the default 8 MHz. s/other/different/ > > Signed-off-by: Philipp Rossak <embed3d@gmail.com> > --- > drivers/media/rc/sunxi-cir.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c > index 97f367b446c4..9bbe55a76860 100644 > --- a/drivers/media/rc/sunxi-cir.c > +++ b/drivers/media/rc/sunxi-cir.c > @@ -72,12 +72,8 @@ > /* CIR_REG register idle threshold */ > #define REG_CIR_ITHR(val) (((val) << 8) & (GENMASK(15, 8))) > > -/* Required frequency for IR0 or IR1 clock in CIR mode */ > +/* Required frequency for IR0 or IR1 clock in CIR mode (default) */ > #define SUNXI_IR_BASE_CLK 8000000 > -/* Frequency after IR internal divider */ > -#define SUNXI_IR_CLK (SUNXI_IR_BASE_CLK / 64) > -/* Sample period in ns */ > -#define SUNXI_IR_SAMPLE (1000000000ul / SUNXI_IR_CLK) > /* Noise threshold in samples */ > #define SUNXI_IR_RXNOISE 1 > /* Idle Threshold in samples */ > @@ -122,7 +118,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id) > /* for each bit in fifo */ > dt = readb(ir->base + SUNXI_IR_RXFIFO_REG); > rawir.pulse = (dt & 0x80) != 0; > - rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE; > + rawir.duration = ((dt & 0x7f) + 1) * ir->rc->rx_resolution; Line over 80 characters. > ir_raw_event_store_with_filter(ir->rc, &rawir); > } > } > @@ -148,6 +144,7 @@ static int sunxi_ir_probe(struct platform_device *pdev) > struct device_node *dn = dev->of_node; > struct resource *res; > struct sunxi_ir *ir; > + u32 b_clk_freq; > > ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL); > if (!ir) > @@ -172,6 +169,11 @@ static int sunxi_ir_probe(struct platform_device *pdev) > return PTR_ERR(ir->clk); > } > > + /* Base clock frequency (optional) */ > + if (of_property_read_u32(dn, "clock-frequency", &b_clk_freq)) { > + b_clk_freq = SUNXI_IR_BASE_CLK; > + } No braces here please; please use ./scripts/checkpatch.pl to find issues like this. > + > /* Reset (optional) */ > ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL); > if (IS_ERR(ir->rst)) > @@ -180,11 +182,12 @@ static int sunxi_ir_probe(struct platform_device *pdev) > if (ret) > return ret; > > - ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK); > + ret = clk_set_rate(ir->clk, b_clk_freq); > if (ret) { > dev_err(dev, "set ir base clock failed!\n"); > goto exit_reset_assert; > } > + dev_info(dev, "set base clock frequency to %d Hz.\n", b_clk_freq); > > if (clk_prepare_enable(ir->apb_clk)) { > dev_err(dev, "try to enable apb_ir_clk failed\n"); > @@ -225,7 +228,8 @@ static int sunxi_ir_probe(struct platform_device *pdev) > ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY; > ir->rc->dev.parent = dev; > ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER; > - ir->rc->rx_resolution = SUNXI_IR_SAMPLE; > + /* Frequency after IR internal divider with sample period in ns */ > + ir->rc->rx_resolution = (1000000000ul / (b_clk_freq / 64)); > ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT); > ir->rc->driver_name = SUNXI_IR_DEV; > > -- > 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Philipp, just a couple of small nitpicks. > + u32 b_clk_freq; [...] > + /* Base clock frequency (optional) */ > + if (of_property_read_u32(dn, "clock-frequency", &b_clk_freq)) { > + b_clk_freq = SUNXI_IR_BASE_CLK; > + } > + how about you intialize 'b_clk_freq' to 'SUNXI_IR_BASE_CLK' and just call 'of_property_read_u32' without if statement. 'b_clk_freq' value should not be changed if "clock-frequency" is not present in the DTS. This might avoid misinterpretation from static analyzers that might think that 'b_clk_freq' is used uninitialized. > + dev_info(dev, "set base clock frequency to %d Hz.\n", b_clk_freq); Please use dev_dbg(). Andi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossak <embed3d@gmail.com> wrote: > The CIR Pin of the A83T is located at PL12. > > Signed-off-by: Philipp Rossak <embed3d@gmail.com> > --- > arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi > index a384b766f3dc..954c2393325f 100644 > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi > @@ -521,6 +521,11 @@ > drive-strength = <20>; > bias-pull-up; > }; > + > + ir_pins_a: ir@0 { ir_pins: ir-pins And it really should be cir, to distinguish it from IrDA. ChenYu > + pins = "PL12"; > + function = "s_cir_rx"; > + }; > }; > > r_rsb: rsb@1f03400 { > -- > 2.11.0 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossak <embed3d@gmail.com> wrote: > The Bananapi M3 has an onboard IR receiver. > This enables the onboard IR receiver subnode. > Other than the other IR receivers this one needs a base clock frequency Unlike the other... > of 3000000 Hz (3 MHz), to be able to work. > > Signed-off-by: Philipp Rossak <embed3d@gmail.com> > --- > arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts > index 6550bf0e594b..2bf25ca64133 100644 > --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts > +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts > @@ -100,6 +100,13 @@ > status = "okay"; > }; > > +&ir { See my other reply about the name. Otherwise, Acked-by: Chen-Yu Tsai <wens@csie.org> > + pinctrl-names = "default"; > + pinctrl-0 = <&ir_pins_a>; > + clock-frequency = <3000000>; > + status = "okay"; > +}; > + > &mdio { > rgmii_phy: ethernet-phy@1 { > compatible = "ethernet-phy-ieee802.3-c22"; > -- > 2.11.0 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Dec 17, 2017 at 11:45:46PM +0100, Philipp Rossak wrote: > The ir interface is like on the H3 located at 0x01f02000 and is exactly > the same. This patch adds support for the ir interface on the A83T. > > Signed-off-by: Philipp Rossak <embed3d@gmail.com> > --- > arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi > index 954c2393325f..9e7ed3b9a6b8 100644 > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi > @@ -503,6 +503,16 @@ > #reset-cells = <1>; > }; > > + ir: ir@01f02000 { > + compatible = "allwinner,sun5i-a13-ir"; > + clocks = <&r_ccu CLK_APB0_IR>, <&r_ccu CLK_IR>; > + clock-names = "apb", "ir"; > + resets = <&r_ccu RST_APB0_IR>; > + interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; > + reg = <0x01f02000 0x40>; The size should be the size of the whole memory block, not just the registers you need. Maxime
Hey Andi, thanks for the feedback. I will fix that in the next version of this patch series! On 18.12.2017 03:44, Andi Shyti wrote: > Hi Philipp, > > just a couple of small nitpicks. > >> + u32 b_clk_freq; > > [...] > >> + /* Base clock frequency (optional) */ >> + if (of_property_read_u32(dn, "clock-frequency", &b_clk_freq)) { >> + b_clk_freq = SUNXI_IR_BASE_CLK; >> + } >> + > > how about you intialize 'b_clk_freq' to 'SUNXI_IR_BASE_CLK' and > just call 'of_property_read_u32' without if statement. > 'b_clk_freq' value should not be changed if "clock-frequency" > is not present in the DTS. > > This might avoid misinterpretation from static analyzers that > might think that 'b_clk_freq' is used uninitialized. > >> + dev_info(dev, "set base clock frequency to %d Hz.\n", b_clk_freq); > > Please use dev_dbg(). > > Andi > -- Philipp -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html