From: Wolfram Sang <wsa@the-dreams.de>
To: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Cc: linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, naveenkrishna.ch@gmail.com,
kgene.kim@samsung.com, grant.likely@secretlab.ca,
linux-kernel@vger.kernel.org, taeggyun.ko@samsung.com,
balbi@ti.com, thomas.abraham@linaro.org
Subject: Re: [PATCH v10] i2c: exynos5: add High Speed I2C controller driver
Date: Mon, 1 Jul 2013 08:17:22 +0200 [thread overview]
Message-ID: <20130701061722.GB2976@katana> (raw)
In-Reply-To: <1371638905-30633-1-git-send-email-ch.naveen@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]
On Wed, Jun 19, 2013 at 04:18:25PM +0530, Naveen Krishna Chatradhi wrote:
> Adds support for High Speed I2C driver found in Exynos5 and
> later SoCs from Samsung.
>
> Driver only supports Device Tree method.
>
> Changes since v1:
> 1. Added FIFO functionality
> 2. Added High speed mode functionality
> 3. Remove SMBUS_QUICK
> 4. Remove the debugfs functionality
> 5. Use devm_* functions where ever possible
> 6. Driver is free from GPIO configs
> 7. Use OF data string "clock-frequency" to get the bus operating frequencies
> 8. Split the clock divisor calculation function
> 9. Add resets for the failed transacton cases
> 10. Removed retries as core does retries if -EAGAIN is returned
> 11. Removed mode from device tree info (use speed to distinguish
> the mode of operation)
> 12. Use wait_for_completion_timeout as the interruptible case is not tested well
> 13. few other bug fixes and cosmetic changes
>
> Signed-off-by: Taekgyun Ko <taeggyun.ko@samsung.com>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Reviewed-by: Simon Glass <sjg@google.com>
> Tested-by: Andrew Bresticker <abrestic@google.com>
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> Signed-off-by: Andrew Bresticker <abrestic@google.com>
> ---
>
> +Optional properties:
> + - clock-frequency: Desired operating frequency in Hz of the bus.
> + -> If not specified, the default value is 100khz in fast-speed mode and
> + 1Mhz in high-speed mode.
? If not specified, the default is 100kHz. There is no way to get 1MHz,
or?
...
> +static int exynos5_i2c_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct exynos5_i2c *i2c;
> + struct resource *mem;
> + unsigned int op_clock;
My compiler says:
drivers/i2c/busses/i2c-exynos5.c: In function ‘exynos5_i2c_probe’:
drivers/i2c/busses/i2c-exynos5.c:687:5: warning: ‘op_clock’ may be used uninitialized in this function [-Wuninitialized]
so...
> + int ret;
> +
> + if (!np) {
> + dev_err(&pdev->dev, "no device node\n");
> + return -ENOENT;
> + }
> +
> + i2c = devm_kzalloc(&pdev->dev, sizeof(struct exynos5_i2c), GFP_KERNEL);
> + if (!i2c) {
> + dev_err(&pdev->dev, "no memory for state\n");
> + return -ENOMEM;
> + }
> +
> + if (of_property_read_u32(np, "clock-frequency", &op_clock)) {
> + i2c->speed_mode = HSI2C_FAST_SPD;
> + i2c->fs_clock = HSI2C_FS_TX_CLOCK;
> + }
> +
> + if (op_clock >= HSI2C_HS_TX_CLOCK) {
... this should be 'else if'
> + i2c->speed_mode = HSI2C_HIGH_SPD;
> + i2c->fs_clock = HSI2C_FS_TX_CLOCK;
> + i2c->hs_clock = op_clock;
> + } else {
> + i2c->speed_mode = HSI2C_FAST_SPD;
> + i2c->fs_clock = op_clock;
> + }
...
> + i2c->bus_id = of_alias_get_id(i2c->adap.dev.of_node, "hsi2c");
Huh, the core already gets an alias for you. Can't you use 'adap.nr'?
Rest looks good.
Thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
To: Naveen Krishna Chatradhi
<ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
naveenkrishna.ch-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
taeggyun.ko-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
balbi-l0cyMroinI0@public.gmane.org,
thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
Subject: Re: [PATCH v10] i2c: exynos5: add High Speed I2C controller driver
Date: Mon, 1 Jul 2013 08:17:22 +0200 [thread overview]
Message-ID: <20130701061722.GB2976@katana> (raw)
In-Reply-To: <1371638905-30633-1-git-send-email-ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3121 bytes --]
On Wed, Jun 19, 2013 at 04:18:25PM +0530, Naveen Krishna Chatradhi wrote:
> Adds support for High Speed I2C driver found in Exynos5 and
> later SoCs from Samsung.
>
> Driver only supports Device Tree method.
>
> Changes since v1:
> 1. Added FIFO functionality
> 2. Added High speed mode functionality
> 3. Remove SMBUS_QUICK
> 4. Remove the debugfs functionality
> 5. Use devm_* functions where ever possible
> 6. Driver is free from GPIO configs
> 7. Use OF data string "clock-frequency" to get the bus operating frequencies
> 8. Split the clock divisor calculation function
> 9. Add resets for the failed transacton cases
> 10. Removed retries as core does retries if -EAGAIN is returned
> 11. Removed mode from device tree info (use speed to distinguish
> the mode of operation)
> 12. Use wait_for_completion_timeout as the interruptible case is not tested well
> 13. few other bug fixes and cosmetic changes
>
> Signed-off-by: Taekgyun Ko <taeggyun.ko-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Simon Glass <sjg-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Tested-by: Andrew Bresticker <abrestic-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Andrew Bresticker <abrestic-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
>
> +Optional properties:
> + - clock-frequency: Desired operating frequency in Hz of the bus.
> + -> If not specified, the default value is 100khz in fast-speed mode and
> + 1Mhz in high-speed mode.
? If not specified, the default is 100kHz. There is no way to get 1MHz,
or?
...
> +static int exynos5_i2c_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct exynos5_i2c *i2c;
> + struct resource *mem;
> + unsigned int op_clock;
My compiler says:
drivers/i2c/busses/i2c-exynos5.c: In function ‘exynos5_i2c_probe’:
drivers/i2c/busses/i2c-exynos5.c:687:5: warning: ‘op_clock’ may be used uninitialized in this function [-Wuninitialized]
so...
> + int ret;
> +
> + if (!np) {
> + dev_err(&pdev->dev, "no device node\n");
> + return -ENOENT;
> + }
> +
> + i2c = devm_kzalloc(&pdev->dev, sizeof(struct exynos5_i2c), GFP_KERNEL);
> + if (!i2c) {
> + dev_err(&pdev->dev, "no memory for state\n");
> + return -ENOMEM;
> + }
> +
> + if (of_property_read_u32(np, "clock-frequency", &op_clock)) {
> + i2c->speed_mode = HSI2C_FAST_SPD;
> + i2c->fs_clock = HSI2C_FS_TX_CLOCK;
> + }
> +
> + if (op_clock >= HSI2C_HS_TX_CLOCK) {
... this should be 'else if'
> + i2c->speed_mode = HSI2C_HIGH_SPD;
> + i2c->fs_clock = HSI2C_FS_TX_CLOCK;
> + i2c->hs_clock = op_clock;
> + } else {
> + i2c->speed_mode = HSI2C_FAST_SPD;
> + i2c->fs_clock = op_clock;
> + }
...
> + i2c->bus_id = of_alias_get_id(i2c->adap.dev.of_node, "hsi2c");
Huh, the core already gets an alias for you. Can't you use 'adap.nr'?
Rest looks good.
Thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: wsa@the-dreams.de (Wolfram Sang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v10] i2c: exynos5: add High Speed I2C controller driver
Date: Mon, 1 Jul 2013 08:17:22 +0200 [thread overview]
Message-ID: <20130701061722.GB2976@katana> (raw)
In-Reply-To: <1371638905-30633-1-git-send-email-ch.naveen@samsung.com>
On Wed, Jun 19, 2013 at 04:18:25PM +0530, Naveen Krishna Chatradhi wrote:
> Adds support for High Speed I2C driver found in Exynos5 and
> later SoCs from Samsung.
>
> Driver only supports Device Tree method.
>
> Changes since v1:
> 1. Added FIFO functionality
> 2. Added High speed mode functionality
> 3. Remove SMBUS_QUICK
> 4. Remove the debugfs functionality
> 5. Use devm_* functions where ever possible
> 6. Driver is free from GPIO configs
> 7. Use OF data string "clock-frequency" to get the bus operating frequencies
> 8. Split the clock divisor calculation function
> 9. Add resets for the failed transacton cases
> 10. Removed retries as core does retries if -EAGAIN is returned
> 11. Removed mode from device tree info (use speed to distinguish
> the mode of operation)
> 12. Use wait_for_completion_timeout as the interruptible case is not tested well
> 13. few other bug fixes and cosmetic changes
>
> Signed-off-by: Taekgyun Ko <taeggyun.ko@samsung.com>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Reviewed-by: Simon Glass <sjg@google.com>
> Tested-by: Andrew Bresticker <abrestic@google.com>
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> Signed-off-by: Andrew Bresticker <abrestic@google.com>
> ---
>
> +Optional properties:
> + - clock-frequency: Desired operating frequency in Hz of the bus.
> + -> If not specified, the default value is 100khz in fast-speed mode and
> + 1Mhz in high-speed mode.
? If not specified, the default is 100kHz. There is no way to get 1MHz,
or?
...
> +static int exynos5_i2c_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct exynos5_i2c *i2c;
> + struct resource *mem;
> + unsigned int op_clock;
My compiler says:
drivers/i2c/busses/i2c-exynos5.c: In function ?exynos5_i2c_probe?:
drivers/i2c/busses/i2c-exynos5.c:687:5: warning: ?op_clock? may be used uninitialized in this function [-Wuninitialized]
so...
> + int ret;
> +
> + if (!np) {
> + dev_err(&pdev->dev, "no device node\n");
> + return -ENOENT;
> + }
> +
> + i2c = devm_kzalloc(&pdev->dev, sizeof(struct exynos5_i2c), GFP_KERNEL);
> + if (!i2c) {
> + dev_err(&pdev->dev, "no memory for state\n");
> + return -ENOMEM;
> + }
> +
> + if (of_property_read_u32(np, "clock-frequency", &op_clock)) {
> + i2c->speed_mode = HSI2C_FAST_SPD;
> + i2c->fs_clock = HSI2C_FS_TX_CLOCK;
> + }
> +
> + if (op_clock >= HSI2C_HS_TX_CLOCK) {
... this should be 'else if'
> + i2c->speed_mode = HSI2C_HIGH_SPD;
> + i2c->fs_clock = HSI2C_FS_TX_CLOCK;
> + i2c->hs_clock = op_clock;
> + } else {
> + i2c->speed_mode = HSI2C_FAST_SPD;
> + i2c->fs_clock = op_clock;
> + }
...
> + i2c->bus_id = of_alias_get_id(i2c->adap.dev.of_node, "hsi2c");
Huh, the core already gets an alias for you. Can't you use 'adap.nr'?
Rest looks good.
Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130701/5b228867/attachment.sig>
next prev parent reply other threads:[~2013-07-01 6:17 UTC|newest]
Thread overview: 113+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-27 13:00 [PATCH 0/3] i2c: Add High speed I2C controller driver for Exynos5 Naveen Krishna Chatradhi
2012-11-27 13:00 ` Naveen Krishna Chatradhi
2012-11-27 13:00 ` [PATCH 1/3] i2c: exynos5: add High Speed I2C controller driver Naveen Krishna Chatradhi
2012-11-27 13:00 ` Naveen Krishna Chatradhi
2012-11-27 13:23 ` Felipe Balbi
2012-11-27 13:23 ` Felipe Balbi
2012-11-27 13:23 ` Felipe Balbi
2012-11-27 13:34 ` Thomas Abraham
2012-11-27 13:34 ` Thomas Abraham
2012-11-27 13:34 ` Thomas Abraham
2012-11-28 4:23 ` Naveen Krishna Ch
2012-11-28 4:23 ` Naveen Krishna Ch
2012-11-28 4:23 ` Naveen Krishna Ch
2012-12-25 11:25 ` [PATCH 1/2] " Naveen Krishna Chatradhi
2012-12-25 11:25 ` Naveen Krishna Chatradhi
2012-12-25 11:25 ` Naveen Krishna Chatradhi
2012-12-25 11:25 ` [PATCH 2/2] i2c-exynos5: add debugfs support for registers Naveen Krishna Chatradhi
2012-12-25 11:25 ` Naveen Krishna Chatradhi
2012-12-25 11:25 ` Naveen Krishna Chatradhi
2012-12-27 22:57 ` Felipe Balbi
2012-12-27 22:57 ` Felipe Balbi
2012-12-27 22:57 ` Felipe Balbi
2012-12-27 22:59 ` Felipe Balbi
2012-12-27 22:59 ` Felipe Balbi
2012-12-27 22:59 ` Felipe Balbi
2012-12-27 22:57 ` [PATCH 1/2] i2c: exynos5: add High Speed I2C controller driver Felipe Balbi
2012-12-27 22:57 ` Felipe Balbi
2012-12-27 22:57 ` Felipe Balbi
2012-12-28 6:42 ` Naveen Krishna Ch
2012-12-28 6:42 ` Naveen Krishna Ch
2012-12-28 11:27 ` [PATCH v3] " Naveen Krishna Chatradhi
2012-12-28 11:27 ` Naveen Krishna Chatradhi
2012-12-28 11:27 ` Naveen Krishna Chatradhi
2012-12-28 16:36 ` Naveen Krishna Ch
2012-12-28 16:36 ` Naveen Krishna Ch
2012-12-28 16:36 ` Naveen Krishna Ch
2013-01-15 6:23 ` Naveen Krishna Ch
2013-01-15 6:23 ` Naveen Krishna Ch
2013-01-23 5:05 ` Naveen Krishna Ch
2013-01-23 5:05 ` Naveen Krishna Ch
2013-01-23 5:05 ` Naveen Krishna Ch
2013-02-01 15:54 ` [PATCH v4] " Naveen Krishna Chatradhi
2013-02-01 15:54 ` Naveen Krishna Chatradhi
2013-02-01 19:29 ` Wolfram Sang
2013-02-01 19:29 ` Wolfram Sang
2013-02-08 13:16 ` Grant Likely
2013-03-12 4:32 ` [PATCH] " Naveen Krishna Chatradhi
[not found] ` <1363062732-27869-1-git-send-email-ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-03-12 12:53 ` Simon Glass
2013-03-12 13:13 ` Simon Glass
2013-03-12 13:13 ` Simon Glass
2013-03-20 16:26 ` Naveen Krishna Ch
2013-03-20 16:26 ` Naveen Krishna Ch
2013-03-20 16:24 ` [RFC: PATCH v5] " Naveen Krishna Chatradhi
2013-03-20 16:24 ` Naveen Krishna Chatradhi
2013-03-25 11:52 ` Yuvaraj CD
2013-03-26 9:23 ` Wolfram Sang
2013-03-28 22:01 ` [PATCH v6] " Naveen Krishna Chatradhi
2013-03-28 23:40 ` Naveen Krishna Chatradhi
2013-03-28 23:40 ` Naveen Krishna Chatradhi
2013-04-05 4:52 ` [PATCH v7] " Naveen Krishna Chatradhi
2013-04-13 4:40 ` Naveen Krishna Ch
2013-04-13 4:40 ` Naveen Krishna Ch
2013-04-16 10:29 ` Wolfram Sang
[not found] ` <CAHfPSqCe_VJMeH3oCDc0CfcmpawMj0hN7_b3ngHDFtDUsPJLsA@mail.gmail.com>
2013-05-01 23:19 ` Fwd: " Naveen Krishna Ch
2013-05-01 23:19 ` Naveen Krishna Ch
2013-05-02 11:27 ` Wolfram Sang
2013-05-02 11:27 ` Wolfram Sang
2013-05-02 11:48 ` Naveen Krishna Ch
2013-05-02 11:48 ` Naveen Krishna Ch
2013-05-07 2:50 ` [PATCH v8] " Naveen Krishna Chatradhi
2013-05-07 2:50 ` Naveen Krishna Chatradhi
2013-05-07 12:06 ` Sachin Kamat
2013-05-17 10:10 ` [PATCH v9] " Naveen Krishna Chatradhi
2013-05-17 10:10 ` Naveen Krishna Chatradhi
2013-05-23 6:29 ` Naveen Krishna Ch
2013-05-23 6:29 ` Naveen Krishna Ch
[not found] ` <1368785452-15140-1-git-send-email-ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-06-10 8:07 ` Naveen Krishna Ch
2013-06-10 8:10 ` Naveen Krishna Ch
2013-06-10 8:10 ` Naveen Krishna Ch
2013-06-10 14:38 ` Wolfram Sang
2013-06-19 10:48 ` [PATCH v10] " Naveen Krishna Chatradhi
2013-06-19 10:48 ` Naveen Krishna Chatradhi
2013-06-19 10:48 ` Naveen Krishna Chatradhi
2013-07-01 6:17 ` Wolfram Sang [this message]
2013-07-01 6:17 ` Wolfram Sang
2013-07-01 6:17 ` Wolfram Sang
2013-07-01 10:25 ` Tomasz Figa
2013-07-01 10:25 ` Tomasz Figa
2013-07-01 10:25 ` Tomasz Figa
2013-08-15 13:12 ` Wolfram Sang
2013-08-15 13:12 ` Wolfram Sang
2013-08-16 4:58 ` Naveen Krishna Ch
2013-08-16 4:58 ` Naveen Krishna Ch
2013-08-16 4:58 ` Naveen Krishna Ch
2013-08-16 7:05 ` Wolfram Sang
2013-08-16 7:05 ` Wolfram Sang
2013-08-16 7:05 ` Wolfram Sang
2013-08-21 9:24 ` [PATCH] " Naveen Krishna Chatradhi
2013-08-21 9:24 ` Naveen Krishna Chatradhi
2013-09-08 17:03 ` Wolfram Sang
2013-09-08 17:03 ` Wolfram Sang
2013-10-11 11:43 ` Naveen Krishna Ch
2013-10-11 11:43 ` Naveen Krishna Ch
2013-10-16 5:30 ` [PATCH v12] " Naveen Krishna Chatradhi
2013-10-25 11:47 ` Naveen Krishna Ch
2013-10-25 11:47 ` Naveen Krishna Ch
2013-11-01 11:35 ` Wolfram Sang
2012-11-27 13:00 ` [PATCH 2/3] ARM: exynos5: Add gate clocks for HS-I2C Naveen Krishna Chatradhi
2012-11-27 13:00 ` Naveen Krishna Chatradhi
2012-11-27 13:00 ` Naveen Krishna Chatradhi
2012-11-27 13:00 ` [PATCH 3/3] arm: exynos5: Add HS-I2C device tree platform information Naveen Krishna Chatradhi
2012-11-27 13:00 ` Naveen Krishna Chatradhi
2012-11-27 13:00 ` Naveen Krishna Chatradhi
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=20130701061722.GB2976@katana \
--to=wsa@the-dreams.de \
--cc=balbi@ti.com \
--cc=ch.naveen@samsung.com \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=grant.likely@secretlab.ca \
--cc=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=naveenkrishna.ch@gmail.com \
--cc=taeggyun.ko@samsung.com \
--cc=thomas.abraham@linaro.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.