From: mchehab@kernel.org (Mauro Carvalho Chehab)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree
Date: Sat, 16 Dec 2017 07:18:55 -0200 [thread overview]
Message-ID: <20171216071855.0ef43f7b@recife.lan> (raw)
In-Reply-To: <20171216024914.7550-2-embed3d@gmail.com>
Em Sat, 16 Dec 2017 03:49:10 +0100
Philipp Rossak <embed3d@gmail.com> escreveu:
Hi Phillip,
This is not a full review of this patchset. I just want to point you
that you should keep supporting existing DT files.
> This patch updates the sunxi-ir driver to set the ir base clock from
> devicetree.
>
> This is neccessary since there are different ir recievers on the
> market, that operate with different frequencys. So this value needs to
> be set depending on the attached receiver.
Please don't break backward compatibility with old DT files. In this
specific case, it seems simple enough to be backward-compatible.
>
> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
> ---
> drivers/media/rc/sunxi-cir.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 97f367b446c4..55b53d6463e9 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -72,12 +72,6 @@
> /* 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 */
> -#define SUNXI_IR_BASE_CLK 8000000
> -/* Frequency after IR internal divider */
> -#define SUNXI_IR_CLK (SUNXI_IR_BASE_CLK / 64)
Keep those to definitions...
> -/* 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 +116,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;
> ir_raw_event_store_with_filter(ir->rc, &rawir);
> }
> }
> @@ -148,6 +142,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 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> return PTR_ERR(ir->clk);
> }
>
> + /* Required frequency for IR0 or IR1 clock in CIR mode */
> + if (of_property_read_u32(dn, "base-clk-frequency", &b_clk_freq)) {
> + dev_err(dev, "failed to get ir base clock frequency.\n");
> + return -ENODATA;
> + }
> +
And here, instead of returning an error, if the property can't be read,
it means it is an older DT file. Just default to SUNXI_IR_BASE_CLK.
This will make it backward-compatible with old DT files that don't have
such property.
Regards,
Mauro
> /* Reset (optional) */
> ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> if (IS_ERR(ir->rst))
> @@ -180,7 +181,7 @@ 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;
> @@ -225,7 +226,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;
>
Thanks,
Mauro
WARNING: multiple messages have this Message-ID (diff)
From: Mauro Carvalho Chehab <mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
wens-jdAy2FN1RRM@public.gmane.org,
linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
sean-hENCXIMQXOg@public.gmane.org,
p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
Subject: Re: [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree
Date: Sat, 16 Dec 2017 07:18:55 -0200 [thread overview]
Message-ID: <20171216071855.0ef43f7b@recife.lan> (raw)
In-Reply-To: <20171216024914.7550-2-embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Em Sat, 16 Dec 2017 03:49:10 +0100
Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> escreveu:
Hi Phillip,
This is not a full review of this patchset. I just want to point you
that you should keep supporting existing DT files.
> This patch updates the sunxi-ir driver to set the ir base clock from
> devicetree.
>
> This is neccessary since there are different ir recievers on the
> market, that operate with different frequencys. So this value needs to
> be set depending on the attached receiver.
Please don't break backward compatibility with old DT files. In this
specific case, it seems simple enough to be backward-compatible.
>
> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/media/rc/sunxi-cir.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 97f367b446c4..55b53d6463e9 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -72,12 +72,6 @@
> /* 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 */
> -#define SUNXI_IR_BASE_CLK 8000000
> -/* Frequency after IR internal divider */
> -#define SUNXI_IR_CLK (SUNXI_IR_BASE_CLK / 64)
Keep those to definitions...
> -/* 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 +116,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;
> ir_raw_event_store_with_filter(ir->rc, &rawir);
> }
> }
> @@ -148,6 +142,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 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> return PTR_ERR(ir->clk);
> }
>
> + /* Required frequency for IR0 or IR1 clock in CIR mode */
> + if (of_property_read_u32(dn, "base-clk-frequency", &b_clk_freq)) {
> + dev_err(dev, "failed to get ir base clock frequency.\n");
> + return -ENODATA;
> + }
> +
And here, instead of returning an error, if the property can't be read,
it means it is an older DT file. Just default to SUNXI_IR_BASE_CLK.
This will make it backward-compatible with old DT files that don't have
such property.
Regards,
Mauro
> /* Reset (optional) */
> ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> if (IS_ERR(ir->rst))
> @@ -180,7 +181,7 @@ 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;
> @@ -225,7 +226,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;
>
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Philipp Rossak <embed3d@gmail.com>
Cc: robh+dt@kernel.org, mark.rutland@arm.com,
maxime.ripard@free-electrons.com, wens@csie.org,
linux@armlinux.org.uk, sean@mess.org, p.zabel@pengutronix.de,
andi.shyti@samsung.com, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com
Subject: Re: [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree
Date: Sat, 16 Dec 2017 07:18:55 -0200 [thread overview]
Message-ID: <20171216071855.0ef43f7b@recife.lan> (raw)
In-Reply-To: <20171216024914.7550-2-embed3d@gmail.com>
Em Sat, 16 Dec 2017 03:49:10 +0100
Philipp Rossak <embed3d@gmail.com> escreveu:
Hi Phillip,
This is not a full review of this patchset. I just want to point you
that you should keep supporting existing DT files.
> This patch updates the sunxi-ir driver to set the ir base clock from
> devicetree.
>
> This is neccessary since there are different ir recievers on the
> market, that operate with different frequencys. So this value needs to
> be set depending on the attached receiver.
Please don't break backward compatibility with old DT files. In this
specific case, it seems simple enough to be backward-compatible.
>
> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
> ---
> drivers/media/rc/sunxi-cir.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 97f367b446c4..55b53d6463e9 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -72,12 +72,6 @@
> /* 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 */
> -#define SUNXI_IR_BASE_CLK 8000000
> -/* Frequency after IR internal divider */
> -#define SUNXI_IR_CLK (SUNXI_IR_BASE_CLK / 64)
Keep those to definitions...
> -/* 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 +116,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;
> ir_raw_event_store_with_filter(ir->rc, &rawir);
> }
> }
> @@ -148,6 +142,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 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> return PTR_ERR(ir->clk);
> }
>
> + /* Required frequency for IR0 or IR1 clock in CIR mode */
> + if (of_property_read_u32(dn, "base-clk-frequency", &b_clk_freq)) {
> + dev_err(dev, "failed to get ir base clock frequency.\n");
> + return -ENODATA;
> + }
> +
And here, instead of returning an error, if the property can't be read,
it means it is an older DT file. Just default to SUNXI_IR_BASE_CLK.
This will make it backward-compatible with old DT files that don't have
such property.
Regards,
Mauro
> /* Reset (optional) */
> ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> if (IS_ERR(ir->rst))
> @@ -180,7 +181,7 @@ 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;
> @@ -225,7 +226,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;
>
Thanks,
Mauro
next prev parent reply other threads:[~2017-12-16 9:18 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-16 2:49 [RFC 0/5] IR support for A83T - sunxi-ir driver update Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 9:18 ` Mauro Carvalho Chehab [this message]
2017-12-16 9:18 ` Mauro Carvalho Chehab
2017-12-16 9:18 ` Mauro Carvalho Chehab
2017-12-16 17:30 ` Philipp Rossak
2017-12-16 17:30 ` Philipp Rossak
2017-12-16 17:30 ` Philipp Rossak
2017-12-16 2:49 ` [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 18:36 ` Rob Herring
2017-12-16 18:36 ` Rob Herring
2017-12-16 18:36 ` Rob Herring
2017-12-16 2:49 ` [RFC 3/5] ARM: dts: sun8i: a83t: Add the ir pin for the A83T Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` [RFC 4/5] ARM: dts: sun8i: a83t: Add support for the ir interface Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` [RFC 5/5] ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
2017-12-16 2:49 ` Philipp Rossak
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=20171216071855.0ef43f7b@recife.lan \
--to=mchehab@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.