All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfram Sang <w.sang@pengutronix.de>
To: Roland Stigge <stigge@antcom.de>
Cc: grant.likely@secretlab.ca, robherring2@gmail.com,
	vitalywool@gmail.com, khali@linux-fr.org, ben-linux@fluff.org,
	rob.herring@calxeda.com, linux-i2c@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org, arm@kernel.org,
	linux-arm-kernel@lists.infradead.org, kevin.wells@nxp.com,
	srinivas.bakki@nxp.com
Subject: Re: [PATCH v5] i2c: Add device tree support to i2c-pnx.c
Date: Fri, 20 Apr 2012 14:09:50 +0200	[thread overview]
Message-ID: <20120420120950.GA15613@pengutronix.de> (raw)
In-Reply-To: <1334913755-10466-1-git-send-email-stigge@antcom.de>

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On Fri, Apr 20, 2012 at 11:22:35AM +0200, Roland Stigge wrote:
> This patch adds device tree support to the pnx-i2c driver by using platform
> resources for memory region and irq and removing dependency on mach includes.
> 
> The following platforms are affected:
> 
> * PNX
> * LPC31xx (WIP)
> * LPC32xx
> 
> The patch is based on a patch by Jon Smirl, working on lpc31xx integration
> 
> Signed-off-by: Roland Stigge <stigge@antcom.de>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>

NACK! I said the approach of dropping "timeout" is OK, but unless I say
"you may add my ack after changing this and that", please do not
automatically add my ack. Doesn't hurt here, since I have to pick it up
anyway.

> Changes since v4:
> * Removed OF timeout property (to be added later when there is consensus about
>   how to call and handle it)
> * Changed clock-frequency example to decimal

Thanks, I just noticed. Please drop (more precise: remove) the
"slave-addr" for the same reasons as "timeout". Doesn't make much sense
anyhow, pnx doesn't have any slave support.

>  	ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
> -			0, pdev->name, alg_data);
> +			  0, pdev->name, alg_data);

I know that checkpatch complains about this, but I'd prefer to avoid
such changes unless the code before was really unreadable.

Regards,

    Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: Roland Stigge <stigge-uj/7R2tJ6VmzQB+pC5nmwQ@public.gmane.org>
Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org,
	robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	kevin.wells-3arQi8VN3Tc@public.gmane.org,
	srinivas.bakki-3arQi8VN3Tc@public.gmane.org
Subject: Re: [PATCH v5] i2c: Add device tree support to i2c-pnx.c
Date: Fri, 20 Apr 2012 14:09:50 +0200	[thread overview]
Message-ID: <20120420120950.GA15613@pengutronix.de> (raw)
In-Reply-To: <1334913755-10466-1-git-send-email-stigge-uj/7R2tJ6VmzQB+pC5nmwQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]

On Fri, Apr 20, 2012 at 11:22:35AM +0200, Roland Stigge wrote:
> This patch adds device tree support to the pnx-i2c driver by using platform
> resources for memory region and irq and removing dependency on mach includes.
> 
> The following platforms are affected:
> 
> * PNX
> * LPC31xx (WIP)
> * LPC32xx
> 
> The patch is based on a patch by Jon Smirl, working on lpc31xx integration
> 
> Signed-off-by: Roland Stigge <stigge-uj/7R2tJ6VmzQB+pC5nmwQ@public.gmane.org>
> Reviewed-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Acked-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

NACK! I said the approach of dropping "timeout" is OK, but unless I say
"you may add my ack after changing this and that", please do not
automatically add my ack. Doesn't hurt here, since I have to pick it up
anyway.

> Changes since v4:
> * Removed OF timeout property (to be added later when there is consensus about
>   how to call and handle it)
> * Changed clock-frequency example to decimal

Thanks, I just noticed. Please drop (more precise: remove) the
"slave-addr" for the same reasons as "timeout". Doesn't make much sense
anyhow, pnx doesn't have any slave support.

>  	ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
> -			0, pdev->name, alg_data);
> +			  0, pdev->name, alg_data);

I know that checkpatch complains about this, but I'd prefer to avoid
such changes unless the code before was really unreadable.

Regards,

    Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: w.sang@pengutronix.de (Wolfram Sang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5] i2c: Add device tree support to i2c-pnx.c
Date: Fri, 20 Apr 2012 14:09:50 +0200	[thread overview]
Message-ID: <20120420120950.GA15613@pengutronix.de> (raw)
In-Reply-To: <1334913755-10466-1-git-send-email-stigge@antcom.de>

On Fri, Apr 20, 2012 at 11:22:35AM +0200, Roland Stigge wrote:
> This patch adds device tree support to the pnx-i2c driver by using platform
> resources for memory region and irq and removing dependency on mach includes.
> 
> The following platforms are affected:
> 
> * PNX
> * LPC31xx (WIP)
> * LPC32xx
> 
> The patch is based on a patch by Jon Smirl, working on lpc31xx integration
> 
> Signed-off-by: Roland Stigge <stigge@antcom.de>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>

NACK! I said the approach of dropping "timeout" is OK, but unless I say
"you may add my ack after changing this and that", please do not
automatically add my ack. Doesn't hurt here, since I have to pick it up
anyway.

> Changes since v4:
> * Removed OF timeout property (to be added later when there is consensus about
>   how to call and handle it)
> * Changed clock-frequency example to decimal

Thanks, I just noticed. Please drop (more precise: remove) the
"slave-addr" for the same reasons as "timeout". Doesn't make much sense
anyhow, pnx doesn't have any slave support.

>  	ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
> -			0, pdev->name, alg_data);
> +			  0, pdev->name, alg_data);

I know that checkpatch complains about this, but I'd prefer to avoid
such changes unless the code before was really unreadable.

Regards,

    Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120420/041b8d48/attachment.sig>

  reply	other threads:[~2012-04-20 12:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-20  9:22 [PATCH v5] i2c: Add device tree support to i2c-pnx.c Roland Stigge
2012-04-20  9:22 ` Roland Stigge
2012-04-20  9:22 ` Roland Stigge
2012-04-20 12:09 ` Wolfram Sang [this message]
2012-04-20 12:09   ` Wolfram Sang
2012-04-20 12:09   ` Wolfram Sang
2012-04-20 13:14   ` Roland Stigge
2012-04-20 13:14     ` Roland Stigge
2012-04-20 13:14     ` Roland Stigge

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=20120420120950.GA15613@pengutronix.de \
    --to=w.sang@pengutronix.de \
    --cc=arm@kernel.org \
    --cc=ben-linux@fluff.org \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=grant.likely@secretlab.ca \
    --cc=kevin.wells@nxp.com \
    --cc=khali@linux-fr.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rob.herring@calxeda.com \
    --cc=robherring2@gmail.com \
    --cc=srinivas.bakki@nxp.com \
    --cc=stigge@antcom.de \
    --cc=vitalywool@gmail.com \
    /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.