Hi Alexander,
although this patch is targeting sunxi ARM based hardware it doesn't have
to be tied to this architecture.
Could you rename the patch to something like [media] rc: add sunxi-ir
driver. This is typical of the commit
headers that you see in this directory.

On 13 May 2014 20:39, Alexander Bersenev <[email protected]> wrote:

> This patch adds driver for sunxi IR controller.
> It is based on Alexsey Shestacov's work based on the original driver
> supplied by Allwinner.
>
> Signed-off-by: Alexander Bersenev <[email protected]>
> Signed-off-by: Alexsey Shestacov <[email protected]>
>
> snip

> @@ -0,0 +1,309 @@
> +/*
> + * Driver for Allwinner sunXi IR controller
> + *
> + * Copyright (C) 2014 Alexsey Shestacov <[email protected]>
> + * Copyright (C) 2014 Alexander Bersenev <[email protected]>
> + *
> + * Based on sun5i-ir.c:
> + * Copyright (C) 2007-2012 Daniel Wang
> + * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <media/rc-core.h>
> +
> +#define SUNXI_IR_DEV "sunxi-ir"
> +
> +/* Registers */
> +/* IR Control */
> +#define SUNXI_IR_CTL_REG      0x00
> +/* Rx Config */
> +#define SUNXI_IR_RXCTL_REG    0x10
> +/* Rx Data */
> +#define SUNXI_IR_RXFIFO_REG   0x20
> +/* Rx Interrupt Enable */
> +#define SUNXI_IR_RXINT_REG    0x2C
> +/* Rx Interrupt Status */
> +#define SUNXI_IR_RXSTA_REG    0x30
> +/* IR Sample Config */
> +#define SUNXI_IR_CIR_REG      0x34
> +
> +/* IR_RXINT_REG Register fields */
> +#define REG_RXINT_ROI_EN      BIT(0) /* Rx FIFO Overflow */
> +#define REG_RXINT_RPEI_EN     BIT(1) /* Rx Packet End */
> +#define REG_RXINT_RAI_EN      BIT(4) /* Rx FIFO Data Available */
> +/* Rx FIFO available byte level */
> +#define REG_RXINT_RAL__MASK   (BIT(8)|BIT(9)|BIT(10)|BIT(11))
> +#define REG_RXINT_RAL__SHIFT  8
> +static inline uint32_t REG_RXINT_RAL(uint16_t val)
> +{
> +       return (val << REG_RXINT_RAL__SHIFT) & REG_RXINT_RAL__MASK;
> +}
> +
>
You should add the other register fields here which are used by this
driver. Try to
minimise the amount of magic numbers in the code.


> +/* Hardware supported fifo size */
> +#define SUNXI_IR_FIFO_SIZE    16
> +/* How many messages in FIFO trigger IRQ */
> +#define TRIGGER_LEVEL         8
> +/* Required frequency for IR0 or IR1 clock in CIR mode */
> +#define SUNXI_IR_BASE_CLK     8000000
>
> snip

> +
> +       /* Enable CIR Mode */
> +       writel(0x3 << 4, ir->base+SUNXI_IR_CTL_REG);
> +
>
For example here is one. This forces the driver to be only used in CIR
mode. If you look
at the rc directory then you will see that cir drivers have cir in their
name. Either rename
this file or consider how other modes for this driver could be
configured(i.e. device tree)


> +       /* Config IR Sample Register */
> +       /* Fsample = clk */
> +       tmp = 0;
> +       /* Set Filter Threshold */
> +       tmp |= (SUNXI_IR_RXFILT & 0x3f) << 2;
> +       /* Set Idle Threshold */
> +       tmp |= (SUNXI_IR_RXIDLE & 0xff) << 8;
> +       writel(tmp, ir->base + SUNXI_IR_CIR_REG);
> +
> +       /* Invert Input Signal */
> +       writel(0x1 << 2, ir->base + SUNXI_IR_RXCTL_REG);
> +
> +       /* Clear All Rx Interrupt Status */
> +       writel(0xff, ir->base + SUNXI_IR_RXSTA_REG);
> +
> +       /* Enable IRQ on overflow, packet end, FIFO available with trigger
> +          level */
> +       writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
> +              REG_RXINT_RAI_EN | REG_RXINT_RAL(TRIGGER_LEVEL - 1),
> +              ir->base + SUNXI_IR_RXINT_REG);
> +
> +       /* Enable IR Module */
> +       tmp = readl(ir->base + SUNXI_IR_CTL_REG);
> +
> +       writel(tmp | 0x3, ir->base + SUNXI_IR_CTL_REG);
>
> please replace all these magic numbers with defines.


> +       dev_info(dev, "initialized sunXi IR driver\n");
> +       return 0;
> +
> +exit_free_dev:
> +       rc_free_device(ir->rc);
> +exit_clkdisable_clk:
> +       clk_disable_unprepare(ir->clk);
> +exit_clkdisable_apb_clk:
>
> snip.
Thanks for doing this....I've added this to my build and will test on my
Mele A1000.

CK

>
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to