Re: [PATCH 00/10] Remove support for TX49xx
On Wed, 6 Jan 2021 21:41:24 +0100, Geert Uytterhoeven wrote: >> > Is that sufficient to keep it? >> >> for me it is. But now we probaly need some reverts then... > > Indeed. Fortunately not all of it, as some removals were TX4938-only. These patches should not break RBTX4927: net: tc35815: Drop support for TX49XX boards spi: txx9: Remove driver mtd: Remove drivers used by TX49xx char: hw_random: Remove tx4939 driver rtc: tx4939: Remove driver ide: tx4938ide: Remove driver And these patches just break audio-support only. dma: tx49 removal ASoC: txx9: Remove driver I think dma and ASoC drivers are hard to maintain now, and can be dropped for basic support for RBTX4927. (TX39 boards does not have audio-support, so dma txx9 driver can be dropped too) --- Atsushi Nemoto
Re: [PATCH 00/10] Remove support for TX49xx
Hi Geert! On Wed, 6 Jan 2021 09:37:11 +0100, Geert Uytterhoeven wrote: > Hi Thomas, > > CC Nemoto-san (de-facto TX49XX maintainer) > > On Tue, Jan 5, 2021 at 3:03 PM Thomas Bogendoerfer > wrote: >> I couldn't find any buyable product other than reference boards using >> TX49xx CPUs. And since nobody showed interest in keeping support for >> it, it's time to remove it. > > I have an RBTX4927 development board in my board farm, boot-test every > bi-weekly renesas-drivers release on it, and fix kernel issues when they > appear. > > Is that sufficient to keep it? It have been about 10 years since last time I see any TX49 board :-) AFAIK Geert is the last user of TX49 SoC. I'm OK with whole TX49xx (and TX39xx) removal if Geert (or any other users) agreed. --- Atsushi Nemoto
Re: [PATCH] MIPS: TXx9: Fix boot crash in free_initmem()
On Wed, 22 May 2019 10:15:35 +0200, Geert Uytterhoeven
wrote:
> On rbtx4927:
...
> As of commit b93ddc4f9156205e ("mips: Reserve memory for the kernel
> image resources"), bootmem_init() no longer reserves the memory below
> the kernel, while prom_free_prom_memory() still frees it.
>
> Fix this by reverting commit b6263ff2d6e58cc2 ("MIPS: TXx9: Implement
> prom_free_prom_memory").
>
> Suggested-by: Serge Semin
> Fixes: b93ddc4f9156205e ("mips: Reserve memory for the kernel image
> resources")
> Signed-off-by: Geert Uytterhoeven
Thank you for fixing this. It looks OK for me.
Reviewed-by: Atsushi Nemoto
---
Atsushi Nemoto
Re: NFS/TCP crashes on MIPS/RBTX4927 in v4.20-rcX (bisected)
On Wed, 5 Dec 2018 14:41:30 +0100, Geert Uytterhoeven
wrote:
> When using these options, I do see a slowdown in early boot, but the issue
> is still there.
Hmm, the NIC of the board is NE2000 variants, so DMA coherency will
not be an issue anyway. So strange ...
The board has a PCI slot. If you had an legacy PCI NIC card, trying
with it might help finding the bug.
> My next guess is an unaligned access not using {get,put}_unaligned(), which
> doesn't seem to work on tx4927, but doesn't cause an exception neither.
IIRC, TX49 can raise an exception on unaligned access.
---
Atsushi Nemoto
Re: NFS/TCP crashes on MIPS/RBTX4927 in v4.20-rcX (bisected)
On Wed, 5 Dec 2018 14:41:30 +0100, Geert Uytterhoeven
wrote:
> When using these options, I do see a slowdown in early boot, but the issue
> is still there.
Hmm, the NIC of the board is NE2000 variants, so DMA coherency will
not be an issue anyway. So strange ...
The board has a PCI slot. If you had an legacy PCI NIC card, trying
with it might help finding the bug.
> My next guess is an unaligned access not using {get,put}_unaligned(), which
> doesn't seem to work on tx4927, but doesn't cause an exception neither.
IIRC, TX49 can raise an exception on unaligned access.
---
Atsushi Nemoto
Re: NFS/TCP crashes on MIPS/RBTX4927 in v4.20-rcX (bisected)
Hi Geert,
On Tue, 4 Dec 2018 14:53:07 +0100, Geert Uytterhoeven
wrote:
> I found similar crashes in a report from 2006, but of course the code
> has changed too much to apply the solution proposed there
> (https://www.linux-mips.org/archives/linux-mips/2006-09/msg00169.html).
>
> Userland is Debian 8 (the last release supporting "old" MIPS).
> My kernel is based on v4.20.0-rc5, but the issue happens with v4.20-rc1,
> too.
>
> However, I noticed it works in v4.19! Hence I've bisected this, to commit
> 277e4ab7d530bf28 ("SUNRPC: Simplify TCP receive code by switching to using
> iterators").
>
> Dropping the ",tcp" part from the nfsroot parameter also fixes the issue.
>
> Given RBTX4926 is little endian, just like my arm/arm64 boards, it's probably
> not an endianness issue. Sparse didn't show anything suspicious before/after
> the guilty commit.
>
> Do you have a clue?
If it was a cache issue, disabling i-cache or d-cache completely might
help understanding the problem. I added TXx9 specific "icdisable" and
"dcdisable" kernel options for debugging long ago.
I hope these options still works correctly with recent kernel but not
sure.
Also, disabling i-cache makes your board VERY slow, of course.
---
Atsushi Nemoto
Re: NFS/TCP crashes on MIPS/RBTX4927 in v4.20-rcX (bisected)
Hi Geert,
On Tue, 4 Dec 2018 14:53:07 +0100, Geert Uytterhoeven
wrote:
> I found similar crashes in a report from 2006, but of course the code
> has changed too much to apply the solution proposed there
> (https://www.linux-mips.org/archives/linux-mips/2006-09/msg00169.html).
>
> Userland is Debian 8 (the last release supporting "old" MIPS).
> My kernel is based on v4.20.0-rc5, but the issue happens with v4.20-rc1,
> too.
>
> However, I noticed it works in v4.19! Hence I've bisected this, to commit
> 277e4ab7d530bf28 ("SUNRPC: Simplify TCP receive code by switching to using
> iterators").
>
> Dropping the ",tcp" part from the nfsroot parameter also fixes the issue.
>
> Given RBTX4926 is little endian, just like my arm/arm64 boards, it's probably
> not an endianness issue. Sparse didn't show anything suspicious before/after
> the guilty commit.
>
> Do you have a clue?
If it was a cache issue, disabling i-cache or d-cache completely might
help understanding the problem. I added TXx9 specific "icdisable" and
"dcdisable" kernel options for debugging long ago.
I hope these options still works correctly with recent kernel but not
sure.
Also, disabling i-cache makes your board VERY slow, of course.
---
Atsushi Nemoto
Re: [PATCH] mips: txx9: fix iounmap related issue
On Thu, 6 Sep 2018 12:19:19 +0800, Ding Xiang wrote: > if device_register return error, iounmap should be called, also iounmap > need to call before put_device. > > Signed-off-by: Ding Xiang > --- > arch/mips/txx9/generic/setup.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Thank you for fixing this long standing issue. It looks OK for me. Reviewed-by: Atsushi Nemoto --- Atsushi Nemoto
Re: [PATCH] mips: txx9: fix iounmap related issue
On Thu, 6 Sep 2018 12:19:19 +0800, Ding Xiang wrote: > if device_register return error, iounmap should be called, also iounmap > need to call before put_device. > > Signed-off-by: Ding Xiang > --- > arch/mips/txx9/generic/setup.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Thank you for fixing this long standing issue. It looks OK for me. Reviewed-by: Atsushi Nemoto --- Atsushi Nemoto
Re: [PATCH V2] mips: txx9: fix resource leak after register fail
On Wed, 5 Sep 2018 19:22:19 +0800, Ding Xiang wrote: > the memory allocated and ioremap address need free after > device_register return error. ... > exit_put: > put_device(>dev); > - return; > +exit_free: > + iounmap(dev->base); > + kfree(dev); This change will break exit_put error path. I think kfree will be called from txx9_device_release by put_device. Please refer James's comment on previous trial: <https://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips=20180305221833.GJ4197%40saruman> --- Atsushi Nemoto
Re: [PATCH V2] mips: txx9: fix resource leak after register fail
On Wed, 5 Sep 2018 19:22:19 +0800, Ding Xiang wrote: > the memory allocated and ioremap address need free after > device_register return error. ... > exit_put: > put_device(>dev); > - return; > +exit_free: > + iounmap(dev->base); > + kfree(dev); This change will break exit_put error path. I think kfree will be called from txx9_device_release by put_device. Please refer James's comment on previous trial: <https://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips=20180305221833.GJ4197%40saruman> --- Atsushi Nemoto
Re: [PATCH] MIPS: TXx9: Modernize printing of kernel messages
On Wed, 7 Dec 2016 10:05:15 +0100, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > - Convert from printk() to pr_*(), > - Add missing continuations, to fix user-visible breakage, > - Drop superfluous casts (u64 has been unsigned long long on all > architectures for many years). Thank you. Good catch. Reviewed-by: Atsushi Nemoto <an...@mba.ocn.ne.jp> --- Atsushi Nemoto
Re: [PATCH] MIPS: TXx9: Modernize printing of kernel messages
On Wed, 7 Dec 2016 10:05:15 +0100, Geert Uytterhoeven wrote: > - Convert from printk() to pr_*(), > - Add missing continuations, to fix user-visible breakage, > - Drop superfluous casts (u64 has been unsigned long long on all > architectures for many years). Thank you. Good catch. Reviewed-by: Atsushi Nemoto --- Atsushi Nemoto
[PATCH] spi: core: make zero length transfer valid again
Zero length transfer becomes invalid since
"spi: core: Validate length of the transfers in message" commit,
but it should be valid to support an odd device, for example, which
requires long delay between chipselect and the first transfer, etc.
Signed-off-by: Atsushi Nemoto
---
drivers/spi/spi.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c2605aa..454a523 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1808,7 +1808,7 @@ static int __spi_validate(struct spi_device *spi, struct
spi_message *message)
{
struct spi_master *master = spi->master;
struct spi_transfer *xfer;
- int w_size, n_words;
+ int w_size;
if (list_empty(>transfers))
return -EINVAL;
@@ -1871,9 +1871,8 @@ static int __spi_validate(struct spi_device *spi, struct
spi_message *message)
else
w_size = 4;
- n_words = xfer->len / w_size;
/* No partial transfers accepted */
- if (!n_words || xfer->len % w_size)
+ if (xfer->len % w_size)
return -EINVAL;
if (xfer->speed_hz && master->min_speed_hz &&
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] spi: core: Validate length of the transfers in message
On Fri, 28 Feb 2014 15:19:20 +0900, Mark Brown wrote: >> I don't object to the whole patch. Validating in spi core is good of >> course, and "xfer->len % w_size" part looks no problem. > >> I just want to keep ways to handle an odd device, for example, which >> requires long delay between chipselect and the first transfer, etc. > > Can you submit a patch adding that support back please? OK, I will send a patch against spi.git/for-next branch. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] spi: core: Validate length of the transfers in message
On Fri, 28 Feb 2014 15:19:20 +0900, Mark Brown broo...@kernel.org wrote: I don't object to the whole patch. Validating in spi core is good of course, and xfer-len % w_size part looks no problem. I just want to keep ways to handle an odd device, for example, which requires long delay between chipselect and the first transfer, etc. Can you submit a patch adding that support back please? OK, I will send a patch against spi.git/for-next branch. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: core: make zero length transfer valid again
Zero length transfer becomes invalid since
spi: core: Validate length of the transfers in message commit,
but it should be valid to support an odd device, for example, which
requires long delay between chipselect and the first transfer, etc.
Signed-off-by: Atsushi Nemoto an...@mba.ocn.ne.jp
---
drivers/spi/spi.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c2605aa..454a523 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1808,7 +1808,7 @@ static int __spi_validate(struct spi_device *spi, struct
spi_message *message)
{
struct spi_master *master = spi-master;
struct spi_transfer *xfer;
- int w_size, n_words;
+ int w_size;
if (list_empty(message-transfers))
return -EINVAL;
@@ -1871,9 +1871,8 @@ static int __spi_validate(struct spi_device *spi, struct
spi_message *message)
else
w_size = 4;
- n_words = xfer-len / w_size;
/* No partial transfers accepted */
- if (!n_words || xfer-len % w_size)
+ if (xfer-len % w_size)
return -EINVAL;
if (xfer-speed_hz master-min_speed_hz
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] spi: core: Validate length of the transfers in message
On Wed, 26 Feb 2014 09:14:07 +0900, Mark Brown wrote: >> + optionally defining short delays after transfers ... using >> the spi_transfer.delay_usecs setting (this delay can be the >> only protocol effect, if the buffer length is zero); > > *sigh* I guess it is valid, though frankly I'm concerned that this > isn't a good idea - it's certainly not going to work reliably given the > need for every driver to open code this, most of them get the delay > stuff wrong. I don't object to the whole patch. Validating in spi core is good of course, and "xfer->len % w_size" part looks no problem. I just want to keep ways to handle an odd device, for example, which requires long delay between chipselect and the first transfer, etc. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] spi: core: Validate length of the transfers in message
On Wed, 26 Feb 2014 09:14:07 +0900, Mark Brown broo...@kernel.org wrote: + optionally defining short delays after transfers ... using the spi_transfer.delay_usecs setting (this delay can be the only protocol effect, if the buffer length is zero); *sigh* I guess it is valid, though frankly I'm concerned that this isn't a good idea - it's certainly not going to work reliably given the need for every driver to open code this, most of them get the delay stuff wrong. I don't object to the whole patch. Validating in spi core is good of course, and xfer-len % w_size part looks no problem. I just want to keep ways to handle an odd device, for example, which requires long delay between chipselect and the first transfer, etc. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] spi: core: Validate length of the transfers in message
On Thu, 20 Feb 2014 12:02:08 +0200, "Ivan T. Ivanov" wrote: > SPI transfer length should be multiple of SPI word size, > where SPI word size should be power-of-two multiple ... > + n_words = xfer->len / w_size; > + /* No partial transfers accepted */ > + if (!n_words || xfer->len % w_size) > + return -EINVAL; Is xfer->len == 0 invalid? Long time ago I have fixed atmel spi driver to support zero length transfer (commit 06719814 atmel_spi: support zero length transfer). According to Documentation/spi/spi-summary, zeto length transfer seems valid. + optionally defining short delays after transfers ... using the spi_transfer.delay_usecs setting (this delay can be the only protocol effect, if the buffer length is zero); --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] spi: core: Validate length of the transfers in message
On Thu, 20 Feb 2014 12:02:08 +0200, Ivan T. Ivanov iiva...@mm-sol.com wrote: SPI transfer length should be multiple of SPI word size, where SPI word size should be power-of-two multiple ... + n_words = xfer-len / w_size; + /* No partial transfers accepted */ + if (!n_words || xfer-len % w_size) + return -EINVAL; Is xfer-len == 0 invalid? Long time ago I have fixed atmel spi driver to support zero length transfer (commit 06719814 atmel_spi: support zero length transfer). According to Documentation/spi/spi-summary, zeto length transfer seems valid. + optionally defining short delays after transfers ... using the spi_transfer.delay_usecs setting (this delay can be the only protocol effect, if the buffer length is zero); --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tx4939: Kill get_tx4939rtc_plat_data() function
On Wed, 19 Dec 2012 17:29:04 +0800, Axel Lin wrote: > Simply use dev_get_drvdata() instead. > > Signed-off-by: Axel Lin > --- > drivers/rtc/rtc-tx4939.c | 21 - > 1 file changed, 8 insertions(+), 13 deletions(-) Hi Axel, Thank you for cleanup, but, a platform driver should use platform_get_drvdata / platform_set_drvdata, no? I think using dev_get_drvdata() in platform_get_drvdata() is an implementation detail of the platform device framework, so we should not depends on such an internal detail. If there was any reason to use dev_get_drvdata, please convert all platform_get_drvdata and platform_set_drvdata to dev_get_drvdata and dev_set_drvdata. We should avoid mixing two APIs. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: tx4939: Kill get_tx4939rtc_plat_data() function
On Wed, 19 Dec 2012 17:29:04 +0800, Axel Lin axel@ingics.com wrote: Simply use dev_get_drvdata() instead. Signed-off-by: Axel Lin axel@ingics.com --- drivers/rtc/rtc-tx4939.c | 21 - 1 file changed, 8 insertions(+), 13 deletions(-) Hi Axel, Thank you for cleanup, but, a platform driver should use platform_get_drvdata / platform_set_drvdata, no? I think using dev_get_drvdata() in platform_get_drvdata() is an implementation detail of the platform device framework, so we should not depends on such an internal detail. If there was any reason to use dev_get_drvdata, please convert all platform_get_drvdata and platform_set_drvdata to dev_get_drvdata and dev_set_drvdata. We should avoid mixing two APIs. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: rs5c348: Fix hour decoding in 12-hour mode
Correct the offset by subtracting 20 from tm_hour before taking
the modulo 12.
Reported-and-tested-by: James Nute
Signed-off-by: Atsushi Nemoto
---
drivers/rtc/rtc-rs5c348.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-rs5c348.c b/drivers/rtc/rtc-rs5c348.c
index 77074cc..fd5c7af 100644
--- a/drivers/rtc/rtc-rs5c348.c
+++ b/drivers/rtc/rtc-rs5c348.c
@@ -122,9 +122,12 @@ rs5c348_rtc_read_time(struct device *dev, struct rtc_time
*tm)
tm->tm_min = bcd2bin(rxbuf[RS5C348_REG_MINS] & RS5C348_MINS_MASK);
tm->tm_hour = bcd2bin(rxbuf[RS5C348_REG_HOURS] & RS5C348_HOURS_MASK);
if (!pdata->rtc_24h) {
- tm->tm_hour %= 12;
- if (rxbuf[RS5C348_REG_HOURS] & RS5C348_BIT_PM)
+ if (rxbuf[RS5C348_REG_HOURS] & RS5C348_BIT_PM) {
+ tm->tm_hour -= 20;
+ tm->tm_hour %= 12;
tm->tm_hour += 12;
+ } else
+ tm->tm_hour %= 12;
}
tm->tm_wday = bcd2bin(rxbuf[RS5C348_REG_WDAY] & RS5C348_WDAY_MASK);
tm->tm_mday = bcd2bin(rxbuf[RS5C348_REG_DAY] & RS5C348_DAY_MASK);
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: rs5c348: Fix hour decoding in 12-hour mode
Correct the offset by subtracting 20 from tm_hour before taking
the modulo 12.
Reported-and-tested-by: James Nute newte...@gmail.com
Signed-off-by: Atsushi Nemoto an...@mba.ocn.ne.jp
---
drivers/rtc/rtc-rs5c348.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-rs5c348.c b/drivers/rtc/rtc-rs5c348.c
index 77074cc..fd5c7af 100644
--- a/drivers/rtc/rtc-rs5c348.c
+++ b/drivers/rtc/rtc-rs5c348.c
@@ -122,9 +122,12 @@ rs5c348_rtc_read_time(struct device *dev, struct rtc_time
*tm)
tm-tm_min = bcd2bin(rxbuf[RS5C348_REG_MINS] RS5C348_MINS_MASK);
tm-tm_hour = bcd2bin(rxbuf[RS5C348_REG_HOURS] RS5C348_HOURS_MASK);
if (!pdata-rtc_24h) {
- tm-tm_hour %= 12;
- if (rxbuf[RS5C348_REG_HOURS] RS5C348_BIT_PM)
+ if (rxbuf[RS5C348_REG_HOURS] RS5C348_BIT_PM) {
+ tm-tm_hour -= 20;
+ tm-tm_hour %= 12;
tm-tm_hour += 12;
+ } else
+ tm-tm_hour %= 12;
}
tm-tm_wday = bcd2bin(rxbuf[RS5C348_REG_WDAY] RS5C348_WDAY_MASK);
tm-tm_mday = bcd2bin(rxbuf[RS5C348_REG_DAY] RS5C348_DAY_MASK);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [spi-devel-general] [PATCH] atmel_spi: support zero length transfer
On Fri, 22 Feb 2008 14:36:53 -0500, Ned Forrester <[EMAIL PROTECTED]> wrote: > Or are you and Atsushi talking about using spi_transfer.delay_usecs > *with* a zero length transfer to effectively put a delay between the > assertion of CS and the start of the first clock? If so, then I guess I > missed the original point. Sorry. Yes. I want to use zero length transfer with delay_usecs to achieve CS-CLK delay for my funny device. And I was too lazy to reject true nop transfer. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [spi-devel-general] [PATCH] atmel_spi: support zero length transfer
On Fri, 22 Feb 2008 14:36:53 -0500, Ned Forrester [EMAIL PROTECTED] wrote: Or are you and Atsushi talking about using spi_transfer.delay_usecs *with* a zero length transfer to effectively put a delay between the assertion of CS and the start of the first clock? If so, then I guess I missed the original point. Sorry. Yes. I want to use zero length transfer with delay_usecs to achieve CS-CLK delay for my funny device. And I was too lazy to reject true nop transfer. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] atmel_spi: support zero length transfer
On Fri, 22 Feb 2008 10:30:31 +0100, Marc Pignat <[EMAIL PROTECTED]> wrote: > > > David, do you think writing 0 bytes is a valid use of this API? > > > > Just a zero byte transfer ... no, though it depends what you mean > > by "valid". (I'm not sure I'd expect all controller drivers to > > reject such requests.) That has no effect on bits-on-the-wire, > > and would make trouble for various DMA engines. > So, the behaviour is undefined, something between 'crash my dma engine', > 'assert chip select and wait some time', or 'do_nothing'... If the driver could not handle zero length transfer, then the driver should reject it (just like unsupported transfer mode). Then the behavior will be 'assert chip select and wait some time' or 'rejected by the driver'. > > And it would probably deserve a mode flag (sigh) unless someone > > can update every master controller driver. > Supporting the zero-len-write means checking and perhpaps updating > each driver for the benefit of having an unknown length delay. > > We should add the delay field in the spi_device, but this means more work. > > Is this kind of device so common that we need to do all that work? or can we > leave it as is (verified to work only with atmel_spi)? I think my case is not so common. But if the driver could support zero length transfer easily, there is no reason to reject it. And if nobody wanted to support zero length transfer on that driver, it would be no reason to update it ;) --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] atmel_spi: support zero length transfer
On Fri, 22 Feb 2008 10:30:31 +0100, Marc Pignat [EMAIL PROTECTED] wrote: David, do you think writing 0 bytes is a valid use of this API? Just a zero byte transfer ... no, though it depends what you mean by valid. (I'm not sure I'd expect all controller drivers to reject such requests.) That has no effect on bits-on-the-wire, and would make trouble for various DMA engines. So, the behaviour is undefined, something between 'crash my dma engine', 'assert chip select and wait some time', or 'do_nothing'... If the driver could not handle zero length transfer, then the driver should reject it (just like unsupported transfer mode). Then the behavior will be 'assert chip select and wait some time' or 'rejected by the driver'. And it would probably deserve a mode flag (sigh) unless someone can update every master controller driver. Supporting the zero-len-write means checking and perhpaps updating each driver for the benefit of having an unknown length delay. We should add the delay field in the spi_device, but this means more work. Is this kind of device so common that we need to do all that work? or can we leave it as is (verified to work only with atmel_spi)? I think my case is not so common. But if the driver could support zero length transfer easily, there is no reason to reject it. And if nobody wanted to support zero length transfer on that driver, it would be no reason to update it ;) --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] macb: Fix speed setting
Excuse me for sending broken mail... On Thu, 21 Feb 2008 15:12:46 +0100, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > I have to admit that even after looking through include/linux/phy.h and > include/linux/mii.h, I don't have the faintest idea what values we can > expect to find in the "speed" field of phydev. The comment above struct > phy_device says that it is used "like in mii_if_info", but struct > mii_if_info doesn't have a "speed" field... > > I'm willing to take your word for it, but some documentation would be > really nice... Well, simple grepping drivers/net/phy enlighten us ;) Anyway, this patch fixes problem with 10Mbps on AT91SAM9260 board. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] macb: Fix speed setting
On Thu, 21 Feb 2008 15:12:46 +0100, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > I have to admit that even after looking through include/linux/phy.h and > include/linux/mii.h, I don't have the faintest idea what values we can > expect to find in the "speed" field of phydev. The comment above struct > phy_device says that it is used "like in mii_if_info", but struct > mii_if_info doesn't have a "speed" field... > > I'm willing to take your word for it, but some documentation would be > really nice... Well, simple grepping drivers/net/phy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] macb: Fix speed setting
Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by conversion to generic PHY layer in kernel 2.6.23. Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]> --- diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 81bf005..1d210ed 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -148,7 +148,7 @@ static void macb_handle_link_change(struct net_device *dev) if (phydev->duplex) reg |= MACB_BIT(FD); - if (phydev->speed) + if (phydev->speed == SPEED_100) reg |= MACB_BIT(SPD); macb_writel(bp, NCFGR, reg); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] macb: Fix speed setting
Fix NCFGR.SPD setting on 10Mbps. This bug was introduced by conversion to generic PHY layer in kernel 2.6.23. Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED] --- diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 81bf005..1d210ed 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -148,7 +148,7 @@ static void macb_handle_link_change(struct net_device *dev) if (phydev-duplex) reg |= MACB_BIT(FD); - if (phydev-speed) + if (phydev-speed == SPEED_100) reg |= MACB_BIT(SPD); macb_writel(bp, NCFGR, reg); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] macb: Fix speed setting
On Thu, 21 Feb 2008 15:12:46 +0100, Haavard Skinnemoen [EMAIL PROTECTED] wrote: I have to admit that even after looking through include/linux/phy.h and include/linux/mii.h, I don't have the faintest idea what values we can expect to find in the speed field of phydev. The comment above struct phy_device says that it is used like in mii_if_info, but struct mii_if_info doesn't have a speed field... I'm willing to take your word for it, but some documentation would be really nice... Well, simple grepping drivers/net/phy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] macb: Fix speed setting
Excuse me for sending broken mail... On Thu, 21 Feb 2008 15:12:46 +0100, Haavard Skinnemoen [EMAIL PROTECTED] wrote: I have to admit that even after looking through include/linux/phy.h and include/linux/mii.h, I don't have the faintest idea what values we can expect to find in the speed field of phydev. The comment above struct phy_device says that it is used like in mii_if_info, but struct mii_if_info doesn't have a speed field... I'm willing to take your word for it, but some documentation would be really nice... Well, simple grepping drivers/net/phy enlighten us ;) Anyway, this patch fixes problem with 10Mbps on AT91SAM9260 board. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] atmel_spi: Fix clock polarity
The atmel_spi driver does not initialize clock polarity correctly
(except for at91rm9200 CS0 channel) in some case.
The atmel_spi driver uses gpio-controlled chipselect. OTOH spi clock
signal is controlled by CSRn.CPOL bit, but this register controls
clock signal correctly only in 'real transfer' duration. At the time
of cs_activate() call, CSRn.CPOL will be initialized correctly, but
the controller do not know which channel is to be used next, so clock
signal will stay at the inactive state of last transfer. If clock
polarity of new transfer and last transfer was differ, new transfer
will start with wrong clock signal state.
For example, if you started SPI MODE 2 or 3 transfer after SPI MODE 0
or 1 transfer, the clock signal state at the assertion of chipselect
will be low. Of course this will violates SPI transfer.
This patch is short term solution for this problem. It makes all
CSRn.CPOL match for the transfer before activating chipselect. For
longer term, the best fix might be to let NPCS0 stay selected
permanently in MR and overwrite CSR0 with to the new slave's settings
before asserting CS.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
Acked-by: Haavard Skinnemoen <[EMAIL PROTECTED]>
---
This patch is already in mm tree as atmel_spi-fix-clock-polarity.patch.
I just rewrite description slightly and add my signed-off and acked-by
from Haavard Skinnemoen.
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 293b7ca..85687aa 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -87,6 +87,16 @@ static void cs_activate(struct atmel_spi *as, struct
spi_device *spi)
unsigned gpio = (unsigned) spi->controller_data;
unsigned active = spi->mode & SPI_CS_HIGH;
u32 mr;
+ int i;
+ u32 csr;
+ u32 cpol = (spi->mode & SPI_CPOL) ? SPI_BIT(CPOL) : 0;
+
+ /* Make sure clock polarity is correct */
+ for (i = 0; i < spi->master->num_chipselect; i++) {
+ csr = spi_readl(as, CSR0 + 4 * i);
+ if ((csr ^ cpol) & SPI_BIT(CPOL))
+ spi_writel(as, CSR0 + 4 * i, csr ^ SPI_BIT(CPOL));
+ }
mr = spi_readl(as, MR);
mr = SPI_BFINS(PCS, ~(1 << spi->chip_select), mr);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] atmel_spi: support zero length transfer
On Wed, 20 Feb 2008 18:55:01 +0100, Marc Pignat <[EMAIL PROTECTED]> wrote: > > A spi transfer with zero length is not invalid. Such transfer can be > > used to achieve delay before first CLK edge after chipselect assertion. > How long will be that delay? My funny custom device requires 100us or so. Unfortunately atmel_spi can not use such a slow bitrate. > If they are really users of that kind of thing, this should be fixed by adding > a "delay_us_before_xfer" field in the struct spi_transfer. Yes, it would be an another way to achieve it. But as long as zero length transfer is legal on this API, I don't want to add other fields. > Have you tested it? I think if you start a transfer with 0 len, the ENDRX bit > will never rise, however, I'm not sure about this. Yes. I tested it on AT91SAM9260 and it seems ENDRX rises soon. Though it can be possible to avoid starting DMA for zero length transfer, I think it is not worth to optimize for such a rare case. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] atmel_spi: support zero length transfer
A spi transfer with zero length is not invalid. Such transfer can be
used to achieve delay before first CLK edge after chipselect assertion.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 293b7ca..5dff5e0 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -606,7 +606,7 @@ static int atmel_spi_transfer(struct spi_device *spi,
struct spi_message *msg)
return -ESHUTDOWN;
list_for_each_entry(xfer, >transfers, transfer_list) {
- if (!(xfer->tx_buf || xfer->rx_buf)) {
+ if (!(xfer->tx_buf || xfer->rx_buf) && xfer->len) {
dev_dbg(>dev, "missing rx or tx buf\n");
return -EINVAL;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: atmel_spi clock polarity
On Wed, 20 Feb 2008 10:34:46 +0100, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > In that case, I think the best fix is to let NPCS0 stay selected > permanently in MR and overwrite CSR0 with to the new slave's settings > before asserting CS. But that's a more complicated change, and I don't > know how it will affect the AT91RM9200 special cases. > > So I suggest we merge your patch for 2.6.25, and try to optimize it > for 2.6.26. I absolutely agree. > David, do you want me to pass on the patch with my signoff or just ask > Andrew to add my Acked-by to the patch already in mm? The patch in mm also lacks my Signed-off line. I had thought Andrew never take a patch without Signed-off line ;) Should I resend my patch with my Signed-off and Haavard's Acked-by? --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: atmel_spi clock polarity
On Wed, 20 Feb 2008 10:34:46 +0100, Haavard Skinnemoen [EMAIL PROTECTED] wrote: In that case, I think the best fix is to let NPCS0 stay selected permanently in MR and overwrite CSR0 with to the new slave's settings before asserting CS. But that's a more complicated change, and I don't know how it will affect the AT91RM9200 special cases. So I suggest we merge your patch for 2.6.25, and try to optimize it for 2.6.26. I absolutely agree. David, do you want me to pass on the patch with my signoff or just ask Andrew to add my Acked-by to the patch already in mm? The patch in mm also lacks my Signed-off line. I had thought Andrew never take a patch without Signed-off line ;) Should I resend my patch with my Signed-off and Haavard's Acked-by? --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] atmel_spi: support zero length transfer
A spi transfer with zero length is not invalid. Such transfer can be
used to achieve delay before first CLK edge after chipselect assertion.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 293b7ca..5dff5e0 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -606,7 +606,7 @@ static int atmel_spi_transfer(struct spi_device *spi,
struct spi_message *msg)
return -ESHUTDOWN;
list_for_each_entry(xfer, msg-transfers, transfer_list) {
- if (!(xfer-tx_buf || xfer-rx_buf)) {
+ if (!(xfer-tx_buf || xfer-rx_buf) xfer-len) {
dev_dbg(spi-dev, missing rx or tx buf\n);
return -EINVAL;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] atmel_spi: support zero length transfer
On Wed, 20 Feb 2008 18:55:01 +0100, Marc Pignat [EMAIL PROTECTED] wrote: A spi transfer with zero length is not invalid. Such transfer can be used to achieve delay before first CLK edge after chipselect assertion. How long will be that delay? My funny custom device requires 100us or so. Unfortunately atmel_spi can not use such a slow bitrate. If they are really users of that kind of thing, this should be fixed by adding a delay_us_before_xfer field in the struct spi_transfer. Yes, it would be an another way to achieve it. But as long as zero length transfer is legal on this API, I don't want to add other fields. Have you tested it? I think if you start a transfer with 0 len, the ENDRX bit will never rise, however, I'm not sure about this. Yes. I tested it on AT91SAM9260 and it seems ENDRX rises soon. Though it can be possible to avoid starting DMA for zero length transfer, I think it is not worth to optimize for such a rare case. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] atmel_spi: Fix clock polarity
The atmel_spi driver does not initialize clock polarity correctly
(except for at91rm9200 CS0 channel) in some case.
The atmel_spi driver uses gpio-controlled chipselect. OTOH spi clock
signal is controlled by CSRn.CPOL bit, but this register controls
clock signal correctly only in 'real transfer' duration. At the time
of cs_activate() call, CSRn.CPOL will be initialized correctly, but
the controller do not know which channel is to be used next, so clock
signal will stay at the inactive state of last transfer. If clock
polarity of new transfer and last transfer was differ, new transfer
will start with wrong clock signal state.
For example, if you started SPI MODE 2 or 3 transfer after SPI MODE 0
or 1 transfer, the clock signal state at the assertion of chipselect
will be low. Of course this will violates SPI transfer.
This patch is short term solution for this problem. It makes all
CSRn.CPOL match for the transfer before activating chipselect. For
longer term, the best fix might be to let NPCS0 stay selected
permanently in MR and overwrite CSR0 with to the new slave's settings
before asserting CS.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
Acked-by: Haavard Skinnemoen [EMAIL PROTECTED]
---
This patch is already in mm tree as atmel_spi-fix-clock-polarity.patch.
I just rewrite description slightly and add my signed-off and acked-by
from Haavard Skinnemoen.
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 293b7ca..85687aa 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -87,6 +87,16 @@ static void cs_activate(struct atmel_spi *as, struct
spi_device *spi)
unsigned gpio = (unsigned) spi-controller_data;
unsigned active = spi-mode SPI_CS_HIGH;
u32 mr;
+ int i;
+ u32 csr;
+ u32 cpol = (spi-mode SPI_CPOL) ? SPI_BIT(CPOL) : 0;
+
+ /* Make sure clock polarity is correct */
+ for (i = 0; i spi-master-num_chipselect; i++) {
+ csr = spi_readl(as, CSR0 + 4 * i);
+ if ((csr ^ cpol) SPI_BIT(CPOL))
+ spi_writel(as, CSR0 + 4 * i, csr ^ SPI_BIT(CPOL));
+ }
mr = spi_readl(as, MR);
mr = SPI_BFINS(PCS, ~(1 spi-chip_select), mr);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: atmel_spi clock polarity
On Mon, 18 Feb 2008 15:31:58 +0100, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > > Anyway, I will try your patch in a few days. > > Ok, thanks. If it works, that would be great, but given your > description above I'm not sure if I dare hope for it. Unfortunately it did not work. The clock state did not change by writing MR register. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [spi-devel-general] atmel_spi clock polarity
On Mon, 18 Feb 2008 23:49:18 +0100, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > > > CLK __|~|___|~~~|___|~~~|___|~~~|___ > > > > ... and at T1 CPOL is changed?? That's wrong. There should > > never be a partial clock period while a chipselect is active. > > While it's inactive, sure -- no chip should care. > > ...but what I'm afraid of is that since we're using GPIO chipselects, > the controller may _think_ that no chip is selected and change the > clock polarity just before it would pull the chipselect low if we were > using automatic chipselects... Yes. That's I suppose. > If that's the case, it would be a bit strange if it actually helped to > initialize all the CSRn registers, but it would also offer us a nice > workaround... I suppose the clock state of the AT91 just follows _last_ transfer. My patch (setting all CSRn.POL for next transfer) was based on that assumption. > Atsushi, do you by any chance have debugging enabled? That would > explain the long delay from CS activation to the change of clock > polarity. Compared to printk() the DMA setup takes almost no time at > all. No, I did not enabled debugging. > If you can confirm that my patch helps, I think that's the one we want > in mainline. Unfortunately I had no time to try it today. Hopefully tomorrow... --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [spi-devel-general] atmel_spi clock polarity
On Mon, 18 Feb 2008 23:49:18 +0100, Haavard Skinnemoen [EMAIL PROTECTED] wrote: CLK __|~|___|~~~|___|~~~|___|~~~|___ ... and at T1 CPOL is changed?? That's wrong. There should never be a partial clock period while a chipselect is active. While it's inactive, sure -- no chip should care. ...but what I'm afraid of is that since we're using GPIO chipselects, the controller may _think_ that no chip is selected and change the clock polarity just before it would pull the chipselect low if we were using automatic chipselects... Yes. That's I suppose. If that's the case, it would be a bit strange if it actually helped to initialize all the CSRn registers, but it would also offer us a nice workaround... I suppose the clock state of the AT91 just follows _last_ transfer. My patch (setting all CSRn.POL for next transfer) was based on that assumption. Atsushi, do you by any chance have debugging enabled? That would explain the long delay from CS activation to the change of clock polarity. Compared to printk() the DMA setup takes almost no time at all. No, I did not enabled debugging. If you can confirm that my patch helps, I think that's the one we want in mainline. Unfortunately I had no time to try it today. Hopefully tomorrow... --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: atmel_spi clock polarity
On Mon, 18 Feb 2008 15:31:58 +0100, Haavard Skinnemoen [EMAIL PROTECTED] wrote: Anyway, I will try your patch in a few days. Ok, thanks. If it works, that would be great, but given your description above I'm not sure if I dare hope for it. Unfortunately it did not work. The clock state did not change by writing MR register. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: atmel_spi clock polarity
On Mon, 18 Feb 2008 12:42:37 +0100, Haavard Skinnemoen <[EMAIL PROTECTED]> wrote: > > Here is my quick workaround for this problem. It makes all CSRn.CPOL > > match for the transfer before activating chipselect. I'm not quite > > sure my analysis is correct, and there might be better solution. > > Could you give me any comments? > > I'm not sure if I fully understand what problem you're seeing. Is the > clock state wrong when the chip select is activated? Yes. > If so, does the patch below help? Hmm... It might fix my problem. But IIRC the clock state follows CSRn.CPOL just before the real transfer. Like this (previous transfer was MODE 0, new transfer is MODE 3): T0T1 T2 CS ~~~| CLK __|~|___|~~~|___|~~~|___|~~~|___ SO ~~|___|~~~|___|~~~|___|~~~|_ MSB T0-T1 was relatively longer then T1-T2. I suppose T1 is not the point of updating MR register, but the point of starting DMA transfer. Anyway, I will try your patch in a few days. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: atmel_spi clock polarity
On Mon, 18 Feb 2008 12:42:37 +0100, Haavard Skinnemoen [EMAIL PROTECTED] wrote: Here is my quick workaround for this problem. It makes all CSRn.CPOL match for the transfer before activating chipselect. I'm not quite sure my analysis is correct, and there might be better solution. Could you give me any comments? I'm not sure if I fully understand what problem you're seeing. Is the clock state wrong when the chip select is activated? Yes. If so, does the patch below help? Hmm... It might fix my problem. But IIRC the clock state follows CSRn.CPOL just before the real transfer. Like this (previous transfer was MODE 0, new transfer is MODE 3): T0T1 T2 CS ~~~| CLK __|~|___|~~~|___|~~~|___|~~~|___ SO ~~|___|~~~|___|~~~|___|~~~|_ MSB T0-T1 was relatively longer then T1-T2. I suppose T1 is not the point of updating MR register, but the point of starting DMA transfer. Anyway, I will try your patch in a few days. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: spi transfer with zero length
On Sat, 16 Feb 2008 10:58:48 -0800, David Brownell <[EMAIL PROTECTED]> wrote: > On Saturday 16 February 2008, Atsushi Nemoto wrote: > > Hi. Is it legal to use zero for 'len' field of struct spi_transfer? > > I mean, len=0, tx_buf=rx_buf=NULL, delay_usecs!=0. > > Yes that should work ... it's uncommon, but not illegal. Some > controller drivers may even handle that right! > > If the delay were zero and cs_change didn't indicate a need to > briefly deselect the chip, it might make sense to reject such > a NOP transfer. But that's not the case you identify. Thank you. I suppose spi-bitbang can handle that right. Then I'm going to fix drivers if it cannot handle such case. > > Some SPI devices need slightly long delay before first CLK edge after > > CS assertion. > > For future reference ... could you identify a few such devices, > and say what "long" is relative to the clock period? > > Some folk have just slowed down the clock in such cases, but > that's rather sub-optimal. Well, the device I have is so special custom one implemented by PIC or something. It needs 100us or so with 200ns clock period. Slowing down might be an another solution but some spi driver cannot use such a slow bitrate (~10KHz). > > To achieve this, I think inserting using a zero length > > transfer before real transfers. But it seems some drivers do not > > handle this case properly. > > Feel free to submit patches fixing those bugs. OK, if I had enough time to make a patch for uptodate version of the driver ;) --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: spi transfer with zero length
On Sat, 16 Feb 2008 10:58:48 -0800, David Brownell [EMAIL PROTECTED] wrote: On Saturday 16 February 2008, Atsushi Nemoto wrote: Hi. Is it legal to use zero for 'len' field of struct spi_transfer? I mean, len=0, tx_buf=rx_buf=NULL, delay_usecs!=0. Yes that should work ... it's uncommon, but not illegal. Some controller drivers may even handle that right! If the delay were zero and cs_change didn't indicate a need to briefly deselect the chip, it might make sense to reject such a NOP transfer. But that's not the case you identify. Thank you. I suppose spi-bitbang can handle that right. Then I'm going to fix drivers if it cannot handle such case. Some SPI devices need slightly long delay before first CLK edge after CS assertion. For future reference ... could you identify a few such devices, and say what long is relative to the clock period? Some folk have just slowed down the clock in such cases, but that's rather sub-optimal. Well, the device I have is so special custom one implemented by PIC or something. It needs 100us or so with 200ns clock period. Slowing down might be an another solution but some spi driver cannot use such a slow bitrate (~10KHz). To achieve this, I think inserting using a zero length transfer before real transfers. But it seems some drivers do not handle this case properly. Feel free to submit patches fixing those bugs. OK, if I had enough time to make a patch for uptodate version of the driver ;) --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
spi transfer with zero length
Hi. Is it legal to use zero for 'len' field of struct spi_transfer? I mean, len=0, tx_buf=rx_buf=NULL, delay_usecs!=0. Some SPI devices need slightly long delay before first CLK edge after CS assertion. To achieve this, I think inserting using a zero length transfer before real transfers. But it seems some drivers do not handle this case properly. Is this driver's bug, or we need additional delay field in struct spi_device for such case? --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
atmel_spi clock polarity
I found that atmel_spi driver does not initialize clock polarity
correctly (except for at91rm9200 CS0 channel) in some case.
The atmel_spi driver uses gpio-controlled chipselect. OTOH spi clock
signal is controlled by CSRn.CPOL bit, but this register controls
clock signal correctly only in 'real transfer' duration. At the time
of cs_activate() call, CSRn.CPOL will be initialized correctly, but
the controller do not know which channel is to be used next, so clock
signal will stay at the inactive state of last transfer. If clock
polarity of new transfer and last transfer was differ, new transfer
will start with wrong clock signal state.
For example, if you started SPI MODE 2 or 3 transfer after SPI MODE 0
or 1 transfer, the clock signal state at the assertion of chipselect
will be low. Of course this will violates SPI transfer.
Here is my quick workaround for this problem. It makes all CSRn.CPOL
match for the transfer before activating chipselect. I'm not quite
sure my analysis is correct, and there might be better solution.
Could you give me any comments?
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 293b7ca..85687aa 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -87,6 +87,16 @@ static void cs_activate(struct atmel_spi *as, struct
spi_device *spi)
unsigned gpio = (unsigned) spi->controller_data;
unsigned active = spi->mode & SPI_CS_HIGH;
u32 mr;
+ int i;
+ u32 csr;
+ u32 cpol = (spi->mode & SPI_CPOL) ? SPI_BIT(CPOL) : 0;
+
+ /* Make sure clock polarity is correct */
+ for (i = 0; i < spi->master->num_chipselect; i++) {
+ csr = spi_readl(as, CSR0 + 4 * i);
+ if ((csr ^ cpol) & SPI_BIT(CPOL))
+ spi_writel(as, CSR0 + 4 * i, csr ^ SPI_BIT(CPOL));
+ }
mr = spi_readl(as, MR);
mr = SPI_BFINS(PCS, ~(1 << spi->chip_select), mr);
---
Atsushi Nemoto
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
atmel_spi clock polarity
I found that atmel_spi driver does not initialize clock polarity
correctly (except for at91rm9200 CS0 channel) in some case.
The atmel_spi driver uses gpio-controlled chipselect. OTOH spi clock
signal is controlled by CSRn.CPOL bit, but this register controls
clock signal correctly only in 'real transfer' duration. At the time
of cs_activate() call, CSRn.CPOL will be initialized correctly, but
the controller do not know which channel is to be used next, so clock
signal will stay at the inactive state of last transfer. If clock
polarity of new transfer and last transfer was differ, new transfer
will start with wrong clock signal state.
For example, if you started SPI MODE 2 or 3 transfer after SPI MODE 0
or 1 transfer, the clock signal state at the assertion of chipselect
will be low. Of course this will violates SPI transfer.
Here is my quick workaround for this problem. It makes all CSRn.CPOL
match for the transfer before activating chipselect. I'm not quite
sure my analysis is correct, and there might be better solution.
Could you give me any comments?
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 293b7ca..85687aa 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -87,6 +87,16 @@ static void cs_activate(struct atmel_spi *as, struct
spi_device *spi)
unsigned gpio = (unsigned) spi-controller_data;
unsigned active = spi-mode SPI_CS_HIGH;
u32 mr;
+ int i;
+ u32 csr;
+ u32 cpol = (spi-mode SPI_CPOL) ? SPI_BIT(CPOL) : 0;
+
+ /* Make sure clock polarity is correct */
+ for (i = 0; i spi-master-num_chipselect; i++) {
+ csr = spi_readl(as, CSR0 + 4 * i);
+ if ((csr ^ cpol) SPI_BIT(CPOL))
+ spi_writel(as, CSR0 + 4 * i, csr ^ SPI_BIT(CPOL));
+ }
mr = spi_readl(as, MR);
mr = SPI_BFINS(PCS, ~(1 spi-chip_select), mr);
---
Atsushi Nemoto
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
spi transfer with zero length
Hi. Is it legal to use zero for 'len' field of struct spi_transfer? I mean, len=0, tx_buf=rx_buf=NULL, delay_usecs!=0. Some SPI devices need slightly long delay before first CLK edge after CS assertion. To achieve this, I think inserting using a zero length transfer before real transfers. But it seems some drivers do not handle this case properly. Is this driver's bug, or we need additional delay field in struct spi_device for such case? --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers
On Mon, 11 Feb 2008 00:16:07 +0100, Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> wrote: > > It is too late for -git but not too late for 2.6.25-rc1. ;-) > > > > Could you make a patch please? > > I went ahead and made a patch (sorry, I needed it for git pull request :). > > From: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> > Subject: [PATCH] ide: ide_init_port() bugfix Thanks! Looks sane for me :) --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers
On Mon, 11 Feb 2008 00:16:07 +0100, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote: It is too late for -git but not too late for 2.6.25-rc1. ;-) Could you make a patch please? I went ahead and made a patch (sorry, I needed it for git pull request :). From: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] Subject: [PATCH] ide: ide_init_port() bugfix Thanks! Looks sane for me :) --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers
On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> wrote: > + /* reset DMA masks only for SFF-style DMA controllers */ > + if ((d->host_flags && IDE_HFLAG_NO_DMA) == 0 && hwif->dma_base == 0) > + hwif->swdma_mask = hwif->mwdma_mask = hwif->ultra_mask = 0; It might be too late, but "host_flags && IDE_HFLAGS_NO_DMA" seems wrong for me. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers
On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote: + /* reset DMA masks only for SFF-style DMA controllers */ + if ((d-host_flags IDE_HFLAG_NO_DMA) == 0 hwif-dma_base == 0) + hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0; It might be too late, but host_flags IDE_HFLAGS_NO_DMA seems wrong for me. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Platform real time clock driver for Dallas 1511 chip.
On Tue, 4 Dec 2007 16:46:51 -0800, Andrew Sharp <[EMAIL PROTECTED]> wrote:
> +struct rtc_plat_data {
> + struct rtc_device *rtc;
> + void __iomem *ioaddr; /* virtual base address */
> + unsigned long baseaddr; /* physical base address */
> + int size; /* amount of memory mapped */
> + int irq;
> + unsigned int irqen;
> + int alrm_sec;
> + int alrm_min;
> + int alrm_hour;
> + int alrm_mday;
> +};
> +
> +static DEFINE_SPINLOCK(ds1511_lock);
> +
> +static __iomem char *ds1511_base;
If you used ds1511_base, ioaddr in rtc_plat_data is not needed.
> + static noinline void
> +rtc_write(uint8_t val, uint32_t reg)
> +{
> + writeb(val, ds1511_base + (reg * reg_spacing));
> +}
Still "noinline" here and there.
> +static struct bin_attribute ds1511_nvram_attr = {
> + .attr = {
> + .name = "nvram",
> + .mode = S_IRUGO | S_IWUGO,
> + .owner = THIS_MODULE,
> + },
> + .size = DS1511_RAM_MAX,
> + .read = ds1511_nvram_read,
> + .write = ds1511_nvram_write,
> +};
It would be better to use S_IWUSR instead of S_IWUGO. Please check
rtc-ds1553 changes in current git tree.
> +static struct platform_driver ds1511_rtc_driver = {
> + .probe = ds1511_rtc_probe,
> + .remove = __devexit_p(ds1511_rtc_remove),
> + .driver = {
> + .name = "ds1511",
> + .owner = THIS_MODULE,
> + },
> +};
Use "rtc-ds1511" for driver name for hotplugging. Please check
rtc-ds1553 changes in current git tree.
> + static void __exit
> +ds1511_rtc_exit(void)
> +{
> + return platform_driver_unregister(_rtc_driver);
> +}
Do not return void value. Please check rtc-ds1553 changes in current
git tree.
---
Atsushi Nemoto
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Platform real time clock driver for Dallas 1511 chip.
On Tue, 4 Dec 2007 16:46:51 -0800, Andrew Sharp [EMAIL PROTECTED] wrote:
+struct rtc_plat_data {
+ struct rtc_device *rtc;
+ void __iomem *ioaddr; /* virtual base address */
+ unsigned long baseaddr; /* physical base address */
+ int size; /* amount of memory mapped */
+ int irq;
+ unsigned int irqen;
+ int alrm_sec;
+ int alrm_min;
+ int alrm_hour;
+ int alrm_mday;
+};
+
+static DEFINE_SPINLOCK(ds1511_lock);
+
+static __iomem char *ds1511_base;
If you used ds1511_base, ioaddr in rtc_plat_data is not needed.
+ static noinline void
+rtc_write(uint8_t val, uint32_t reg)
+{
+ writeb(val, ds1511_base + (reg * reg_spacing));
+}
Still noinline here and there.
+static struct bin_attribute ds1511_nvram_attr = {
+ .attr = {
+ .name = nvram,
+ .mode = S_IRUGO | S_IWUGO,
+ .owner = THIS_MODULE,
+ },
+ .size = DS1511_RAM_MAX,
+ .read = ds1511_nvram_read,
+ .write = ds1511_nvram_write,
+};
It would be better to use S_IWUSR instead of S_IWUGO. Please check
rtc-ds1553 changes in current git tree.
+static struct platform_driver ds1511_rtc_driver = {
+ .probe = ds1511_rtc_probe,
+ .remove = __devexit_p(ds1511_rtc_remove),
+ .driver = {
+ .name = ds1511,
+ .owner = THIS_MODULE,
+ },
+};
Use rtc-ds1511 for driver name for hotplugging. Please check
rtc-ds1553 changes in current git tree.
+ static void __exit
+ds1511_rtc_exit(void)
+{
+ return platform_driver_unregister(ds1511_rtc_driver);
+}
Do not return void value. Please check rtc-ds1553 changes in current
git tree.
---
Atsushi Nemoto
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: heartbeat for leds on i2c bus
On Mon, 3 Dec 2007 11:35:57 +0100, Rodolfo Giometti <[EMAIL PROTECTED]> wrote: > If I try to use the heartbeat trigger I hang the system... I suppose > it is due the fact the led_set_brightness() function is called inside > a kernel timer. > > How I can solve this problem? I thought to modify the trigger adding a > kernel thread who manage the led which in turn is controlled by the > kernel timer. It could be the right solution? Yes, the heartbeat trigger driver assumes brightness_set() routine can be called from timer. Other led-triggers also have this assumption. So I'd suggest using workqueue for your led driver, as like as the leds-gpio driver. Or you can just use the leds-gpio driver and implement the gpio interface (gpio_set_value_cansleep) instead of the led interface. --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: heartbeat for leds on i2c bus
On Mon, 3 Dec 2007 11:35:57 +0100, Rodolfo Giometti [EMAIL PROTECTED] wrote: If I try to use the heartbeat trigger I hang the system... I suppose it is due the fact the led_set_brightness() function is called inside a kernel timer. How I can solve this problem? I thought to modify the trigger adding a kernel thread who manage the led which in turn is controlled by the kernel timer. It could be the right solution? Yes, the heartbeat trigger driver assumes brightness_set() routine can be called from timer. Other led-triggers also have this assumption. So I'd suggest using workqueue for your led driver, as like as the leds-gpio driver. Or you can just use the leds-gpio driver and implement the gpio interface (gpio_set_value_cansleep) instead of the led interface. --- Atsushi Nemoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: Fix error paths on txx9spi_probe
Some error paths in txx9spi_probe wrongly return 0. This patch fixes
them by using the devres interfaces.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
drivers/spi/spi_txx9.c | 40
1 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/spi/spi_txx9.c b/drivers/spi/spi_txx9.c
index cc5094f..363ac8e 100644
--- a/drivers/spi/spi_txx9.c
+++ b/drivers/spi/spi_txx9.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
@@ -74,7 +75,6 @@ struct txx9spi {
struct list_head queue;
wait_queue_head_t waitq;
void __iomem *membase;
- int irq;
int baseclk;
struct clk *clk;
u32 max_speed_hz, min_speed_hz;
@@ -350,12 +350,12 @@ static int __init txx9spi_probe(struct platform_device
*dev)
struct resource *res;
int ret = -ENODEV;
u32 mcr;
+ int irq;
master = spi_alloc_master(>dev, sizeof(*c));
if (!master)
return ret;
c = spi_master_get_devdata(master);
- c->irq = -1;
platform_set_drvdata(dev, master);
INIT_WORK(>work, txx9spi_work);
@@ -381,32 +381,36 @@ static int __init txx9spi_probe(struct platform_device
*dev)
res = platform_get_resource(dev, IORESOURCE_MEM, 0);
if (!res)
- goto exit;
- c->membase = ioremap(res->start, res->end - res->start + 1);
+ goto exit_busy;
+ if (!devm_request_mem_region(>dev,
+res->start, res->end - res->start + 1,
+"spi_txx9"))
+ goto exit_busy;
+ c->membase = devm_ioremap(>dev,
+ res->start, res->end - res->start + 1);
if (!c->membase)
- goto exit;
+ goto exit_busy;
/* enter config mode */
mcr = txx9spi_rd(c, TXx9_SPMCR);
mcr &= ~(TXx9_SPMCR_OPMODE | TXx9_SPMCR_SPSTP | TXx9_SPMCR_BCLR);
txx9spi_wr(c, mcr | TXx9_SPMCR_CONFIG | TXx9_SPMCR_BCLR, TXx9_SPMCR);
- c->irq = platform_get_irq(dev, 0);
- if (c->irq < 0)
- goto exit;
- ret = request_irq(c->irq, txx9spi_interrupt, 0, dev->name, c);
- if (ret) {
- c->irq = -1;
+ irq = platform_get_irq(dev, 0);
+ if (irq < 0)
+ goto exit_busy;
+ ret = devm_request_irq(>dev, irq, txx9spi_interrupt, 0,
+ "spi_txx9", c);
+ if (ret)
goto exit;
- }
c->workqueue =
create_singlethread_workqueue(master->dev.parent->bus_id);
if (!c->workqueue)
- goto exit;
+ goto exit_busy;
c->last_chipselect = -1;
dev_info(>dev, "at %#llx, irq %d, %dMHz\n",
-(unsigned long long)res->start, c->irq,
+(unsigned long long)res->start, irq,
(c->baseclk + 50) / 100);
master->bus_num = dev->id;
@@ -418,13 +422,11 @@ static int __init txx9spi_probe(struct platform_device
*dev)
if (ret)
goto exit;
return 0;
+exit_busy:
+ ret = -EBUSY;
exit:
if (c->workqueue)
destroy_workqueue(c->workqueue);
- if (c->irq >= 0)
- free_irq(c->irq, c);
- if (c->membase)
- iounmap(c->membase);
if (c->clk) {
clk_disable(c->clk);
clk_put(c->clk);
@@ -442,8 +444,6 @@ static int __exit txx9spi_remove(struct platform_device
*dev)
spi_unregister_master(master);
platform_set_drvdata(dev, NULL);
destroy_workqueue(c->workqueue);
- free_irq(c->irq, c);
- iounmap(c->membase);
clk_disable(c->clk);
clk_put(c->clk);
spi_master_put(master);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: Fix double-free on spi_unregister_master
After 49dce689ad4ef0fd1f970ef762168e4bd46f69a3, device_for_each_child
iteration hits the master device itself. Do not call
spi_unregister_device() for the master device.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
drivers/spi/spi.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 89769ce..b31f443 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -457,10 +457,11 @@ done:
EXPORT_SYMBOL_GPL(spi_register_master);
-static int __unregister(struct device *dev, void *unused)
+static int __unregister(struct device *dev, void *master_dev)
{
/* note: before about 2.6.14-rc1 this would corrupt memory: */
- spi_unregister_device(to_spi_device(dev));
+ if (dev != master_dev)
+ spi_unregister_device(to_spi_device(dev));
return 0;
}
@@ -478,7 +479,8 @@ void spi_unregister_master(struct spi_master *master)
{
int dummy;
- dummy = device_for_each_child(master->dev.parent, NULL, __unregister);
+ dummy = device_for_each_child(master->dev.parent, >dev,
+ __unregister);
device_unregister(>dev);
}
EXPORT_SYMBOL_GPL(spi_unregister_master);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: Fix error paths on txx9spi_probe
Some error paths in txx9spi_probe wrongly return 0. This patch fixes
them by using the devres interfaces.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
drivers/spi/spi_txx9.c | 40
1 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/spi/spi_txx9.c b/drivers/spi/spi_txx9.c
index cc5094f..363ac8e 100644
--- a/drivers/spi/spi_txx9.c
+++ b/drivers/spi/spi_txx9.c
@@ -24,6 +24,7 @@
#include linux/spi/spi.h
#include linux/err.h
#include linux/clk.h
+#include linux/io.h
#include asm/gpio.h
@@ -74,7 +75,6 @@ struct txx9spi {
struct list_head queue;
wait_queue_head_t waitq;
void __iomem *membase;
- int irq;
int baseclk;
struct clk *clk;
u32 max_speed_hz, min_speed_hz;
@@ -350,12 +350,12 @@ static int __init txx9spi_probe(struct platform_device
*dev)
struct resource *res;
int ret = -ENODEV;
u32 mcr;
+ int irq;
master = spi_alloc_master(dev-dev, sizeof(*c));
if (!master)
return ret;
c = spi_master_get_devdata(master);
- c-irq = -1;
platform_set_drvdata(dev, master);
INIT_WORK(c-work, txx9spi_work);
@@ -381,32 +381,36 @@ static int __init txx9spi_probe(struct platform_device
*dev)
res = platform_get_resource(dev, IORESOURCE_MEM, 0);
if (!res)
- goto exit;
- c-membase = ioremap(res-start, res-end - res-start + 1);
+ goto exit_busy;
+ if (!devm_request_mem_region(dev-dev,
+res-start, res-end - res-start + 1,
+spi_txx9))
+ goto exit_busy;
+ c-membase = devm_ioremap(dev-dev,
+ res-start, res-end - res-start + 1);
if (!c-membase)
- goto exit;
+ goto exit_busy;
/* enter config mode */
mcr = txx9spi_rd(c, TXx9_SPMCR);
mcr = ~(TXx9_SPMCR_OPMODE | TXx9_SPMCR_SPSTP | TXx9_SPMCR_BCLR);
txx9spi_wr(c, mcr | TXx9_SPMCR_CONFIG | TXx9_SPMCR_BCLR, TXx9_SPMCR);
- c-irq = platform_get_irq(dev, 0);
- if (c-irq 0)
- goto exit;
- ret = request_irq(c-irq, txx9spi_interrupt, 0, dev-name, c);
- if (ret) {
- c-irq = -1;
+ irq = platform_get_irq(dev, 0);
+ if (irq 0)
+ goto exit_busy;
+ ret = devm_request_irq(dev-dev, irq, txx9spi_interrupt, 0,
+ spi_txx9, c);
+ if (ret)
goto exit;
- }
c-workqueue =
create_singlethread_workqueue(master-dev.parent-bus_id);
if (!c-workqueue)
- goto exit;
+ goto exit_busy;
c-last_chipselect = -1;
dev_info(dev-dev, at %#llx, irq %d, %dMHz\n,
-(unsigned long long)res-start, c-irq,
+(unsigned long long)res-start, irq,
(c-baseclk + 50) / 100);
master-bus_num = dev-id;
@@ -418,13 +422,11 @@ static int __init txx9spi_probe(struct platform_device
*dev)
if (ret)
goto exit;
return 0;
+exit_busy:
+ ret = -EBUSY;
exit:
if (c-workqueue)
destroy_workqueue(c-workqueue);
- if (c-irq = 0)
- free_irq(c-irq, c);
- if (c-membase)
- iounmap(c-membase);
if (c-clk) {
clk_disable(c-clk);
clk_put(c-clk);
@@ -442,8 +444,6 @@ static int __exit txx9spi_remove(struct platform_device
*dev)
spi_unregister_master(master);
platform_set_drvdata(dev, NULL);
destroy_workqueue(c-workqueue);
- free_irq(c-irq, c);
- iounmap(c-membase);
clk_disable(c-clk);
clk_put(c-clk);
spi_master_put(master);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: Fix double-free on spi_unregister_master
After 49dce689ad4ef0fd1f970ef762168e4bd46f69a3, device_for_each_child
iteration hits the master device itself. Do not call
spi_unregister_device() for the master device.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
drivers/spi/spi.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 89769ce..b31f443 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -457,10 +457,11 @@ done:
EXPORT_SYMBOL_GPL(spi_register_master);
-static int __unregister(struct device *dev, void *unused)
+static int __unregister(struct device *dev, void *master_dev)
{
/* note: before about 2.6.14-rc1 this would corrupt memory: */
- spi_unregister_device(to_spi_device(dev));
+ if (dev != master_dev)
+ spi_unregister_device(to_spi_device(dev));
return 0;
}
@@ -478,7 +479,8 @@ void spi_unregister_master(struct spi_master *master)
{
int dummy;
- dummy = device_for_each_child(master-dev.parent, NULL, __unregister);
+ dummy = device_for_each_child(master-dev.parent, master-dev,
+ __unregister);
device_unregister(master-dev);
}
EXPORT_SYMBOL_GPL(spi_unregister_master);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2.6.24-rc1] serial: Fix serial_txx9 console initialization
Since commit 97d97224ff361e08777fb33e0fd193ca877dac28, serial_core
calls ->pm() on initialization even if the port was used for console.
This behaviour breaks serial_txx9 console since The serial_txx9 driver
initialize its port entirely on its ->pm() method if new state was 0.
This patch adds checking for oldstate value to fix this probelm.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). In this case do not initialize the
+* port now, because the port was already initialized (for
+* non-console port) or should not be initialized here (for
+* console port). If we initialized the port here we lose
+* serial console settings.
+*/
+ if (state == 0 && oldstate != -1)
serial_txx9_initialize(port);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2.6.24-rc1] serial: Fix serial_txx9 console initialization
Since commit 97d97224ff361e08777fb33e0fd193ca877dac28, serial_core
calls -pm() on initialization even if the port was used for console.
This behaviour breaks serial_txx9 console since The serial_txx9 driver
initialize its port entirely on its -pm() method if new state was 0.
This patch adds checking for oldstate value to fix this probelm.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). In this case do not initialize the
+* port now, because the port was already initialized (for
+* non-console port) or should not be initialized here (for
+* console port). If we initialized the port here we lose
+* serial console settings.
+*/
+ if (state == 0 oldstate != -1)
serial_txx9_initialize(port);
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: do not call ->pm() on initialization for console port
On Thu, 25 Oct 2007 13:40:44 -0700, Andrew Morton <[EMAIL PROTECTED]> wrote:
> > Now rc1 is released. Any chances for this patch?
> > If none, I should push other workaround to fix this issue on 2.6.24.
>
> I don't know what patch you're referring to. I don't appear to have
> whatever-patch-this-is queued up anywhere?
Here is an original patch I proposed (http://lkml.org/lkml/2007/10/18/292)
--
Subject: serial: do not call ->pm() on initialization for console port
From: Atsushi Nemoto <[EMAIL PROTECTED]>
If the port was already enabled as a serial console, there is no need
to call pm callback to power it up. This patch fixes serial console
initialization on serial_txx9 driver.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 68aa4da..5a38076 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2326,7 +2326,6 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
}
state->port = port;
- state->pm_state = -1;
port->cons = drv->cons;
port->info = state->info;
@@ -2336,6 +2335,7 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
* initialised.
*/
if (!(uart_console(port) && (port->cons->flags & CON_ENABLED))) {
+ state->pm_state = -1;
spin_lock_init(>lock);
lockdep_set_class(>lock, _lock_key);
}
On Thu, 25 Oct 2007 22:36:44 +0100, Russell King <[EMAIL PROTECTED]> wrote:
> "Further investigation" is quite a good indication that a patch needs to
> be worked on, no? In any case, I'm ill at the moment, so things are very
> much on hold.
OK, then I'd like to propose this alternative
(http://lkml.org/lkml/2007/10/18/301). This should be work regardless
of the patch above.
----------
Subject: serial: Fix serial_txx9 console initialization
From: Atsushi Nemoto <[EMAIL PROTECTED]>
Now serial_core calls ->pm() on initialization even if the port was
used for console. This behaviour breaks serial_txx9 console since The
serial_txx9 driver initialize its port entirely on its ->pm() method
if new state was 0. This patch adds checking for oldstate value to
fix this probelm.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). In this case do not initialize the
+* port now, because the port was already initialized (for
+* non-console port) or should not be initialized here (for
+* console port). If we initialized the port here we lose
+* serial console settings.
+*/
+ if (state == 0 && oldstate != -1)
serial_txx9_initialize(port);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: do not call ->pm() on initialization for console port
On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <[EMAIL PROTECTED]> wrote: > > Attached patch fixes this problem. Is this a right way? > > If not, I will fix the serial_txx9 driver to skip the initialization in > > ->pm() if oldstate was -1. > > I'll have to investigate further to refresh myself with what's going > on here, but your patch looks wrong. Now rc1 is released. Any chances for this patch? If none, I should push other workaround to fix this issue on 2.6.24. --- Atsushi Nemoto - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: do not call -pm() on initialization for console port
On Thu, 18 Oct 2007 17:18:27 +0100, Russell King [EMAIL PROTECTED] wrote: Attached patch fixes this problem. Is this a right way? If not, I will fix the serial_txx9 driver to skip the initialization in -pm() if oldstate was -1. I'll have to investigate further to refresh myself with what's going on here, but your patch looks wrong. Now rc1 is released. Any chances for this patch? If none, I should push other workaround to fix this issue on 2.6.24. --- Atsushi Nemoto - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: do not call -pm() on initialization for console port
On Thu, 25 Oct 2007 13:40:44 -0700, Andrew Morton [EMAIL PROTECTED] wrote:
Now rc1 is released. Any chances for this patch?
If none, I should push other workaround to fix this issue on 2.6.24.
I don't know what patch you're referring to. I don't appear to have
whatever-patch-this-is queued up anywhere?
Here is an original patch I proposed (http://lkml.org/lkml/2007/10/18/292)
--
Subject: serial: do not call -pm() on initialization for console port
From: Atsushi Nemoto [EMAIL PROTECTED]
If the port was already enabled as a serial console, there is no need
to call pm callback to power it up. This patch fixes serial console
initialization on serial_txx9 driver.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 68aa4da..5a38076 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2326,7 +2326,6 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
}
state-port = port;
- state-pm_state = -1;
port-cons = drv-cons;
port-info = state-info;
@@ -2336,6 +2335,7 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
* initialised.
*/
if (!(uart_console(port) (port-cons-flags CON_ENABLED))) {
+ state-pm_state = -1;
spin_lock_init(port-lock);
lockdep_set_class(port-lock, port_lock_key);
}
On Thu, 25 Oct 2007 22:36:44 +0100, Russell King [EMAIL PROTECTED] wrote:
Further investigation is quite a good indication that a patch needs to
be worked on, no? In any case, I'm ill at the moment, so things are very
much on hold.
OK, then I'd like to propose this alternative
(http://lkml.org/lkml/2007/10/18/301). This should be work regardless
of the patch above.
--
Subject: serial: Fix serial_txx9 console initialization
From: Atsushi Nemoto [EMAIL PROTECTED]
Now serial_core calls -pm() on initialization even if the port was
used for console. This behaviour breaks serial_txx9 console since The
serial_txx9 driver initialize its port entirely on its -pm() method
if new state was 0. This patch adds checking for oldstate value to
fix this probelm.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). In this case do not initialize the
+* port now, because the port was already initialized (for
+* non-console port) or should not be initialized here (for
+* console port). If we initialized the port here we lose
+* serial console settings.
+*/
+ if (state == 0 oldstate != -1)
serial_txx9_initialize(port);
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clocksource: Make CLOCKSOURCE_MASK bullet-proof
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 107787a..07b4215 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -103,7 +103,7 @@ struct clocksource {
#define CLOCK_SOURCE_VALID_FOR_HRES0x20
/* simplify initialization of mask field */
-#define CLOCKSOURCE_MASK(bits) (cycle_t)(bits<64 ? ((1ULL<http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clocksource: Make CLOCKSOURCE_MASK bullet-proof
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 107787a..07b4215 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -103,7 +103,7 @@ struct clocksource {
#define CLOCK_SOURCE_VALID_FOR_HRES0x20
/* simplify initialization of mask field */
-#define CLOCKSOURCE_MASK(bits) (cycle_t)(bits64 ? ((1ULLbits)-1) : -1)
+#define CLOCKSOURCE_MASK(bits) (cycle_t)((bits) 64 ? ((1ULL(bits))-1) : -1)
/**
* clocksource_khz2mult - calculates mult from khz and shift
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: do not call ->pm() on initialization for console port
On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <[EMAIL PROTECTED]> wrote:
> > Attached patch fixes this problem. Is this a right way?
> > If not, I will fix the serial_txx9 driver to skip the initialization in
> > ->pm() if oldstate was -1.
>
> I'll have to investigate further to refresh myself with what's going
> on here, but your patch looks wrong.
Thanks. JFYI, here is some patches I tried before changing
serial_core. Both fixes the serial console problem.
Patch 1:
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). In this case do not initialize the
+* port now, because the port was already initialized (for
+* non-console port) or should not be initialized here (for
+* console port). If we initialized the port here we lose
+* serial console settings.
+*/
+ if (state == 0 && oldstate != -1)
serial_txx9_initialize(port);
}
Patch 2:
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..167c3a7 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,6 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). If we initialized the port here we
+* lose serial console settings.
+*/
+#ifdef CONFIG_SERIAL_TXX9_CONSOLE
+ if (port->line == port->cons->index && oldstate == -1)
+ return;
+#endif
if (state == 0)
serial_txx9_initialize(port);
}
@@ -742,12 +751,7 @@ static void serial_txx9_config_port(struct uart_port
*port, int uflags)
return;
port->type = PORT_TXX9;
up->port.fifosize = TXX9_SIO_TX_FIFO;
-
-#ifdef CONFIG_SERIAL_TXX9_CONSOLE
- if (up->port.line == up->port.cons->index)
- return;
-#endif
- serial_txx9_initialize(port);
+ /* This port will be initialized in subsequent serial_txx9_pm() */
}
static const char *
Hope these patches help your refreshing.
---
Atsushi Nemoto
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: do not call ->pm() on initialization for console port
This commit in current git tree broke serial console for serial_txx9 driver.
> commit 97d97224ff361e08777fb33e0fd193ca877dac28
> Author: Russell King <[EMAIL PROTECTED]>
> Date: Sat Sep 1 21:25:09 2007 +0100
>
> [SERIAL] Fix console initialisation ordering
The serial_txx9 driver initialize its port entirely on its ->pm()
method if new state was 0. With the commit, serial_core calls ->pm()
even if the port was used for console.
Attached patch fixes this problem. Is this a right way?
If not, I will fix the serial_txx9 driver to skip the initialization in
->pm() if oldstate was -1.
--
Subject: serial: do not call ->pm() on initialization for console port
From: Atsushi Nemoto <[EMAIL PROTECTED]>
If the port was already enabled as a serial console, there is no need
to call pm callback to power it up. This patch fixes serial console
initialization on serial_txx9 driver.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 68aa4da..5a38076 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2326,7 +2326,6 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
}
state->port = port;
- state->pm_state = -1;
port->cons = drv->cons;
port->info = state->info;
@@ -2336,6 +2335,7 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
* initialised.
*/
if (!(uart_console(port) && (port->cons->flags & CON_ENABLED))) {
+ state->pm_state = -1;
spin_lock_init(>lock);
lockdep_set_class(>lock, _lock_key);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: do not call -pm() on initialization for console port
This commit in current git tree broke serial console for serial_txx9 driver.
commit 97d97224ff361e08777fb33e0fd193ca877dac28
Author: Russell King [EMAIL PROTECTED]
Date: Sat Sep 1 21:25:09 2007 +0100
[SERIAL] Fix console initialisation ordering
The serial_txx9 driver initialize its port entirely on its -pm()
method if new state was 0. With the commit, serial_core calls -pm()
even if the port was used for console.
Attached patch fixes this problem. Is this a right way?
If not, I will fix the serial_txx9 driver to skip the initialization in
-pm() if oldstate was -1.
--
Subject: serial: do not call -pm() on initialization for console port
From: Atsushi Nemoto [EMAIL PROTECTED]
If the port was already enabled as a serial console, there is no need
to call pm callback to power it up. This patch fixes serial console
initialization on serial_txx9 driver.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 68aa4da..5a38076 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2326,7 +2326,6 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
}
state-port = port;
- state-pm_state = -1;
port-cons = drv-cons;
port-info = state-info;
@@ -2336,6 +2335,7 @@ int uart_add_one_port(struct uart_driver *drv, struct
uart_port *port)
* initialised.
*/
if (!(uart_console(port) (port-cons-flags CON_ENABLED))) {
+ state-pm_state = -1;
spin_lock_init(port-lock);
lockdep_set_class(port-lock, port_lock_key);
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: do not call -pm() on initialization for console port
On Thu, 18 Oct 2007 17:18:27 +0100, Russell King [EMAIL PROTECTED] wrote:
Attached patch fixes this problem. Is this a right way?
If not, I will fix the serial_txx9 driver to skip the initialization in
-pm() if oldstate was -1.
I'll have to investigate further to refresh myself with what's going
on here, but your patch looks wrong.
Thanks. JFYI, here is some patches I tried before changing
serial_core. Both fixes the serial console problem.
Patch 1:
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). In this case do not initialize the
+* port now, because the port was already initialized (for
+* non-console port) or should not be initialized here (for
+* console port). If we initialized the port here we lose
+* serial console settings.
+*/
+ if (state == 0 oldstate != -1)
serial_txx9_initialize(port);
}
Patch 2:
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..167c3a7 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,6 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
+ /*
+* If oldstate was -1 this is called from
+* uart_configure_port(). If we initialized the port here we
+* lose serial console settings.
+*/
+#ifdef CONFIG_SERIAL_TXX9_CONSOLE
+ if (port-line == port-cons-index oldstate == -1)
+ return;
+#endif
if (state == 0)
serial_txx9_initialize(port);
}
@@ -742,12 +751,7 @@ static void serial_txx9_config_port(struct uart_port
*port, int uflags)
return;
port-type = PORT_TXX9;
up-port.fifosize = TXX9_SIO_TX_FIFO;
-
-#ifdef CONFIG_SERIAL_TXX9_CONSOLE
- if (up-port.line == up-port.cons-index)
- return;
-#endif
- serial_txx9_initialize(port);
+ /* This port will be initialized in subsequent serial_txx9_pm() */
}
static const char *
Hope these patches help your refreshing.
---
Atsushi Nemoto
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/21] KGDB: This adds basic support to the MIPS architecture
On Mon, 15 Oct 2007 13:33:07 -0500, Jason Wessel <[EMAIL PROTECTED]> wrote:
> --- /dev/null
> +++ b/drivers/serial/serial_txx9_kgdb.c
> @@ -0,0 +1,152 @@
Thank you for attempting to mainline it. I have some cosmetic comments.
> +static inline unsigned int sio_in(struct uart_port *port, int offset)
> +{
> + return *(volatile u32 *)(port->membase + offset);
> +}
> +
> +static inline void sio_out(struct uart_port *port, int offset,
> + unsigned int value)
> +{
> + *(volatile u32 *)(port->membase + offset) = value;
> +}
It would be better to avoid volatile.
static inline unsigned int sio_in(struct uart_port *port, int offset)
{
return __raw_readl(port->membase + offset);
}
static inline void sio_out(struct uart_port *port, int offset,
unsigned int value)
{
__raw_writel(value, port->membase + offset);
}
> + /* Reset the UART. */
> + sio_out(kgdb_port, TXX9_SIFCR, TXX9_SIFCR_SWRST);
> +#ifdef CONFIG_CPU_TX49XX
> + /*
> + * TX4925 BUG WORKAROUND. Accessing SIOC register
> + * immediately after soft reset causes bus error.
> + */
> + iob();
> + udelay(1);
> +#endif
> + /* Wait until reset is complete. */
> + while (sio_in(kgdb_port, TXX9_SIFCR) & TXX9_SIFCR_SWRST);
A while ago, this part of serial_txx9.c was changed a bit (no ifdefs,
avoid endless loop). Could you change it to:
unsigned int tmout = 1;
sio_out(kgdb_port, TXX9_SIFCR, TXX9_SIFCR_SWRST);
/* TX4925 BUG WORKAROUND. Accessing SIOC register
* immediately after soft reset causes bus error. */
mmiowb();
udelay(1);
while ((sio_in(kgdb_port, TXX9_SIFCR) & TXX9_SIFCR_SWRST) && --tmout)
udelay(1);
> + /* Select the input clock prescaler that fits the baud rate. */
> + quot = (kgdb_port->uartclk + 8 * kgdb_txx9_baud) /
> + (16 * kgdb_txx9_baud);
uart_get_divisor() can be used here:
quot = uart_get_divisor(kgdb_port, kgdb_txx9_baud);
> + if (quot < (256 << 1))
> + sibgr = (quot >> 1) | TXX9_SIBGR_BCLK_T0;
> + else if (quot < ( 256 << 3))
> + sibgr = (quot >> 3) | TXX9_SIBGR_BCLK_T2;
> + else if (quot < ( 256 << 5))
> + sibgr = (quot >> 5) | TXX9_SIBGR_BCLK_T4;
> + else if (quot < ( 256 << 7))
> + sibgr = (quot >> 7) | TXX9_SIBGR_BCLK_T6;
> + else
> + sibgr = 0xff | TXX9_SIBGR_BCLK_T6;
It would be better to do it more similar to serial_txx9.c:
quot >>= 1;
if (quot < 256)
sibgr = quot | TXX9_SIBGR_BCLK_T0;
else if (quot < (256 << 2))
sibgr = (quot >> 2) | TXX9_SIBGR_BCLK_T2;
else if (quot < (256 << 4))
sibgr = (quot >> 4) | TXX9_SIBGR_BCLK_T4;
else if (quot < (256 << 6))
sibgr = (quot >> 6) | TXX9_SIBGR_BCLK_T6;
else
sibgr = 0xff | TXX9_SIBGR_BCLK_T6;
---
Atsushi Nemoto
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/21] KGDB: This adds basic support to the MIPS architecture
On Mon, 15 Oct 2007 13:33:07 -0500, Jason Wessel [EMAIL PROTECTED] wrote:
--- /dev/null
+++ b/drivers/serial/serial_txx9_kgdb.c
@@ -0,0 +1,152 @@
Thank you for attempting to mainline it. I have some cosmetic comments.
+static inline unsigned int sio_in(struct uart_port *port, int offset)
+{
+ return *(volatile u32 *)(port-membase + offset);
+}
+
+static inline void sio_out(struct uart_port *port, int offset,
+ unsigned int value)
+{
+ *(volatile u32 *)(port-membase + offset) = value;
+}
It would be better to avoid volatile.
static inline unsigned int sio_in(struct uart_port *port, int offset)
{
return __raw_readl(port-membase + offset);
}
static inline void sio_out(struct uart_port *port, int offset,
unsigned int value)
{
__raw_writel(value, port-membase + offset);
}
+ /* Reset the UART. */
+ sio_out(kgdb_port, TXX9_SIFCR, TXX9_SIFCR_SWRST);
+#ifdef CONFIG_CPU_TX49XX
+ /*
+ * TX4925 BUG WORKAROUND. Accessing SIOC register
+ * immediately after soft reset causes bus error.
+ */
+ iob();
+ udelay(1);
+#endif
+ /* Wait until reset is complete. */
+ while (sio_in(kgdb_port, TXX9_SIFCR) TXX9_SIFCR_SWRST);
A while ago, this part of serial_txx9.c was changed a bit (no ifdefs,
avoid endless loop). Could you change it to:
unsigned int tmout = 1;
sio_out(kgdb_port, TXX9_SIFCR, TXX9_SIFCR_SWRST);
/* TX4925 BUG WORKAROUND. Accessing SIOC register
* immediately after soft reset causes bus error. */
mmiowb();
udelay(1);
while ((sio_in(kgdb_port, TXX9_SIFCR) TXX9_SIFCR_SWRST) --tmout)
udelay(1);
+ /* Select the input clock prescaler that fits the baud rate. */
+ quot = (kgdb_port-uartclk + 8 * kgdb_txx9_baud) /
+ (16 * kgdb_txx9_baud);
uart_get_divisor() can be used here:
quot = uart_get_divisor(kgdb_port, kgdb_txx9_baud);
+ if (quot (256 1))
+ sibgr = (quot 1) | TXX9_SIBGR_BCLK_T0;
+ else if (quot ( 256 3))
+ sibgr = (quot 3) | TXX9_SIBGR_BCLK_T2;
+ else if (quot ( 256 5))
+ sibgr = (quot 5) | TXX9_SIBGR_BCLK_T4;
+ else if (quot ( 256 7))
+ sibgr = (quot 7) | TXX9_SIBGR_BCLK_T6;
+ else
+ sibgr = 0xff | TXX9_SIBGR_BCLK_T6;
It would be better to do it more similar to serial_txx9.c:
quot = 1;
if (quot 256)
sibgr = quot | TXX9_SIBGR_BCLK_T0;
else if (quot (256 2))
sibgr = (quot 2) | TXX9_SIBGR_BCLK_T2;
else if (quot (256 4))
sibgr = (quot 4) | TXX9_SIBGR_BCLK_T4;
else if (quot (256 6))
sibgr = (quot 6) | TXX9_SIBGR_BCLK_T6;
else
sibgr = 0xff | TXX9_SIBGR_BCLK_T6;
---
Atsushi Nemoto
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial_txx9: Use UPF_FIXED_PORT
The UPF_FIXED_PORT flags was introduced in 2.6.22 and it can be used
instead of the driver specific verify_port routine.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
drivers/serial/serial_txx9.c | 26 +-
1 files changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 0930e2a..c25c9a5 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -37,7 +37,7 @@
#include
-static char *serial_version = "1.10";
+static char *serial_version = "1.11";
static char *serial_name = "TX39/49 Serial driver";
#define PASS_LIMIT 256
@@ -68,8 +68,6 @@ static char *serial_name = "TX39/49 Serial driver";
*/
#define UART_NR CONFIG_SERIAL_TXX9_NR_UARTS
-#define HIGH_BITS_OFFSET ((sizeof(long)-sizeof(int))*8)
-
struct uart_txx9_port {
struct uart_portport;
/* No additional info for now */
@@ -756,21 +754,6 @@ static void serial_txx9_config_port(struct uart_port
*port, int uflags)
serial_txx9_initialize(port);
}
-static int
-serial_txx9_verify_port(struct uart_port *port, struct serial_struct *ser)
-{
- unsigned long new_port = ser->port;
- if (HIGH_BITS_OFFSET)
- new_port += (unsigned long)ser->port_high << HIGH_BITS_OFFSET;
- if (ser->type != port->type ||
- ser->irq != port->irq ||
- ser->io_type != port->iotype ||
- new_port != port->iobase ||
- (unsigned long)ser->iomem_base != port->mapbase)
- return -EINVAL;
- return 0;
-}
-
static const char *
serial_txx9_type(struct uart_port *port)
{
@@ -794,7 +777,6 @@ static struct uart_ops serial_txx9_pops = {
.release_port = serial_txx9_release_port,
.request_port = serial_txx9_request_port,
.config_port= serial_txx9_config_port,
- .verify_port= serial_txx9_verify_port,
};
static struct uart_txx9_port serial_txx9_ports[UART_NR];
@@ -950,7 +932,8 @@ int __init early_serial_txx9_setup(struct uart_port *port)
serial_txx9_ports[port->line].port = *port;
serial_txx9_ports[port->line].port.ops = _txx9_pops;
- serial_txx9_ports[port->line].port.flags |= UPF_BOOT_AUTOCONF;
+ serial_txx9_ports[port->line].port.flags |=
+ UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
return 0;
}
@@ -995,7 +978,8 @@ static int __devinit serial_txx9_register_port(struct
uart_port *port)
uart->port.irq = port->irq;
uart->port.uartclk = port->uartclk;
uart->port.iotype = port->iotype;
- uart->port.flags= port->flags | UPF_BOOT_AUTOCONF;
+ uart->port.flags= port->flags
+ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
uart->port.mapbase = port->mapbase;
if (port->dev)
uart->port.dev = port->dev;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial_txx9: Use UPF_FIXED_PORT
The UPF_FIXED_PORT flags was introduced in 2.6.22 and it can be used
instead of the driver specific verify_port routine.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
drivers/serial/serial_txx9.c | 26 +-
1 files changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 0930e2a..c25c9a5 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -37,7 +37,7 @@
#include asm/io.h
-static char *serial_version = 1.10;
+static char *serial_version = 1.11;
static char *serial_name = TX39/49 Serial driver;
#define PASS_LIMIT 256
@@ -68,8 +68,6 @@ static char *serial_name = TX39/49 Serial driver;
*/
#define UART_NR CONFIG_SERIAL_TXX9_NR_UARTS
-#define HIGH_BITS_OFFSET ((sizeof(long)-sizeof(int))*8)
-
struct uart_txx9_port {
struct uart_portport;
/* No additional info for now */
@@ -756,21 +754,6 @@ static void serial_txx9_config_port(struct uart_port
*port, int uflags)
serial_txx9_initialize(port);
}
-static int
-serial_txx9_verify_port(struct uart_port *port, struct serial_struct *ser)
-{
- unsigned long new_port = ser-port;
- if (HIGH_BITS_OFFSET)
- new_port += (unsigned long)ser-port_high HIGH_BITS_OFFSET;
- if (ser-type != port-type ||
- ser-irq != port-irq ||
- ser-io_type != port-iotype ||
- new_port != port-iobase ||
- (unsigned long)ser-iomem_base != port-mapbase)
- return -EINVAL;
- return 0;
-}
-
static const char *
serial_txx9_type(struct uart_port *port)
{
@@ -794,7 +777,6 @@ static struct uart_ops serial_txx9_pops = {
.release_port = serial_txx9_release_port,
.request_port = serial_txx9_request_port,
.config_port= serial_txx9_config_port,
- .verify_port= serial_txx9_verify_port,
};
static struct uart_txx9_port serial_txx9_ports[UART_NR];
@@ -950,7 +932,8 @@ int __init early_serial_txx9_setup(struct uart_port *port)
serial_txx9_ports[port-line].port = *port;
serial_txx9_ports[port-line].port.ops = serial_txx9_pops;
- serial_txx9_ports[port-line].port.flags |= UPF_BOOT_AUTOCONF;
+ serial_txx9_ports[port-line].port.flags |=
+ UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
return 0;
}
@@ -995,7 +978,8 @@ static int __devinit serial_txx9_register_port(struct
uart_port *port)
uart-port.irq = port-irq;
uart-port.uartclk = port-uartclk;
uart-port.iotype = port-iotype;
- uart-port.flags= port-flags | UPF_BOOT_AUTOCONF;
+ uart-port.flags= port-flags
+ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
uart-port.mapbase = port-mapbase;
if (port-dev)
uart-port.dev = port-dev;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: rtc-ds1742.c should use resource_size_t for base address
On Fri, 14 Sep 2007 15:54:27 +1000, David Gibson <[EMAIL PROTECTED]> wrote: > Currently the rtc driver, rtc-ds1742.c uses an unsigned long to store > the base mmio address of the NVRAM/RTC. This breaks on systems like > PowerPC 440, which is a 32-bit core with 36-bit physical addresses: IO > on the system, including the RTC, is typically above the 4GB point, > and cannot fit into an unsigned long. > > This patch fixes the problem by replacing the unsigned long with a > resource_size_t. Tested on Ebony (PPC440) (with additional patches to > instantiate the ds1742 platform device appropriately). > > Signed-off-by: David Gibson <[EMAIL PROTECTED]> Thanks! Acked-by: Atsushi Nemoto <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: rtc-ds1553.c should use resource_size_t for base address
Currently the rtc driver, rtc-ds1552.c uses an unsigned long to store
the base mmio address of the NVRAM/RTC. This breaks on 32-bit systems
with larger physical addresses.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
index 46da571..5ab3492 100644
--- a/drivers/rtc/rtc-ds1553.c
+++ b/drivers/rtc/rtc-ds1553.c
@@ -61,7 +61,7 @@
struct rtc_plat_data {
struct rtc_device *rtc;
void __iomem *ioaddr;
- unsigned long baseaddr;
+ resource_size_t baseaddr;
unsigned long last_jiffies;
int irq;
unsigned int irqen;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: rtc-ds1553.c should use resource_size_t for base address
Currently the rtc driver, rtc-ds1552.c uses an unsigned long to store
the base mmio address of the NVRAM/RTC. This breaks on 32-bit systems
with larger physical addresses.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
index 46da571..5ab3492 100644
--- a/drivers/rtc/rtc-ds1553.c
+++ b/drivers/rtc/rtc-ds1553.c
@@ -61,7 +61,7 @@
struct rtc_plat_data {
struct rtc_device *rtc;
void __iomem *ioaddr;
- unsigned long baseaddr;
+ resource_size_t baseaddr;
unsigned long last_jiffies;
int irq;
unsigned int irqen;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: rtc-ds1742.c should use resource_size_t for base address
On Fri, 14 Sep 2007 15:54:27 +1000, David Gibson [EMAIL PROTECTED] wrote: Currently the rtc driver, rtc-ds1742.c uses an unsigned long to store the base mmio address of the NVRAM/RTC. This breaks on systems like PowerPC 440, which is a 32-bit core with 36-bit physical addresses: IO on the system, including the RTC, is typically above the 4GB point, and cannot fit into an unsigned long. This patch fixes the problem by replacing the unsigned long with a resource_size_t. Tested on Ebony (PPC440) (with additional patches to instantiate the ds1742 platform device appropriately). Signed-off-by: David Gibson [EMAIL PROTECTED] Thanks! Acked-by: Atsushi Nemoto [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: Make rtc-ds1742 driver hotplug-aware
On Sun, 19 Aug 2007 19:02:04 -0700, David Brownell <[EMAIL PROTECTED]> wrote: > This is indeed a platform driver. And v2 of this patch > doesn't resolve its "won't hotplug" problem. The simplest > way to resolve that would be switching to the more widely > used platform_device_register(). Yes, if the current in-tree users (JMR3927 and RBTX4927 board) really wanted to hotplug this driver. And I believe they won't :-) I just want to fix a potential problem in rtc-ds1742 driver for someone who want to hotplug it. That's all. --- Atsushi Nemoto - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: legacy platform drivers and hotplugging
On Sun, 19 Aug 2007 18:44:43 -0700, David Brownell <[EMAIL PROTECTED]> wrote: > Notice I fixed $SUBJECT to not mention rtc-ds1742, which has > never been a platform device and thus is offtopic to your > specific complaints about platform bus hotplugging. Well, rtc-ds1742 *IS* a platform driver. But it seems the discussion is not DS1742 specific at all, and I guess ds1742 is not a good example for hotplugging, so I welcome your $SUBJECT fix :-) --- Atsushi Nemoto - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: Make rtc-ds1742 driver hotplug-aware (take 2)
The rtc-ds1742 platform driver name doesn't match its module name,
which might prevents it from properly hotplugging. There is only two
in-tree user of its driver, which are fixed by this patch too.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
arch/mips/jmr3927/rbhma3100/setup.c|2 +-
.../toshiba_rbtx4927/toshiba_rbtx4927_setup.c |2 +-
drivers/rtc/rtc-ds1742.c |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/jmr3927/rbhma3100/setup.c
b/arch/mips/jmr3927/rbhma3100/setup.c
index fde56e8..ffae163 100644
--- a/arch/mips/jmr3927/rbhma3100/setup.c
+++ b/arch/mips/jmr3927/rbhma3100/setup.c
@@ -427,7 +427,7 @@ static int __init jmr3927_rtc_init(void)
.flags = IORESOURCE_MEM,
};
struct platform_device *dev;
- dev = platform_device_register_simple("ds1742", -1, , 1);
+ dev = platform_device_register_simple("rtc-ds1742", -1, , 1);
return IS_ERR(dev) ? PTR_ERR(dev) : 0;
}
device_initcall(jmr3927_rtc_init);
diff --git a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
index 3e84237..91bd3d1 100644
--- a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
+++ b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
@@ -969,7 +969,7 @@ static int __init toshiba_rbtx4927_rtc_init(void)
.flags = IORESOURCE_MEM,
};
struct platform_device *dev =
- platform_device_register_simple("ds1742", -1, , 1);
+ platform_device_register_simple("rtc-ds1742", -1, , 1);
return IS_ERR(dev) ? PTR_ERR(dev) : 0;
}
device_initcall(toshiba_rbtx4927_rtc_init);
diff --git a/drivers/rtc/rtc-ds1742.c b/drivers/rtc/rtc-ds1742.c
index b2e5481..5562830 100644
--- a/drivers/rtc/rtc-ds1742.c
+++ b/drivers/rtc/rtc-ds1742.c
@@ -251,7 +251,7 @@ static struct platform_driver ds1742_rtc_driver = {
.probe = ds1742_rtc_probe,
.remove = __devexit_p(ds1742_rtc_remove),
.driver = {
- .name = "ds1742",
+ .name = "rtc-ds1742",
.owner = THIS_MODULE,
},
};
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: Make rtc-rs5c348 driver hotplug-aware (take 2)
The rtc-rs5c348 SPI driver name doesn't match its module name, which
prevents it from properly hotplugging. There is only one in-tree user
of its driver, which is fixed by this patch too.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
arch/mips/tx4938/toshiba_rbtx4938/setup.c |2 +-
drivers/rtc/rtc-rs5c348.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/tx4938/toshiba_rbtx4938/setup.c
b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
index 57f3c70..84ebff7 100644
--- a/arch/mips/tx4938/toshiba_rbtx4938/setup.c
+++ b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
@@ -1115,7 +1115,7 @@ static void __init txx9_spi_init(unsigned long base, int
irq)
static int __init rbtx4938_spi_init(void)
{
struct spi_board_info srtc_info = {
- .modalias = "rs5c348",
+ .modalias = "rtc-rs5c348",
.max_speed_hz = 100, /* 1.0Mbps @ Vdd 2.0V */
.bus_num = 0,
.chip_select = 16 + SRTC_CS,
diff --git a/drivers/rtc/rtc-rs5c348.c b/drivers/rtc/rtc-rs5c348.c
index f50f3fc..8394626 100644
--- a/drivers/rtc/rtc-rs5c348.c
+++ b/drivers/rtc/rtc-rs5c348.c
@@ -226,7 +226,7 @@ static int __devexit rs5c348_remove(struct spi_device *spi)
static struct spi_driver rs5c348_driver = {
.driver = {
- .name = "rs5c348",
+ .name = "rtc-rs5c348",
.bus= _bus_type,
.owner = THIS_MODULE,
},
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: Make rtc-ds1553 driver hotplug-aware (take 3)
The rtc-ds1553 platform driver name doesn't match its module name,
which might prevent it from properly hotplugging. This driver has no
in-tree users.
Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
---
The "take 2" of this patch was just a typo-fix ("1553" vs. "1742").
diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
index 46da571..a4bcb34 100644
--- a/drivers/rtc/rtc-ds1553.c
+++ b/drivers/rtc/rtc-ds1553.c
@@ -395,7 +395,7 @@ static struct platform_driver ds1553_rtc_driver = {
.probe = ds1553_rtc_probe,
.remove = __devexit_p(ds1553_rtc_remove),
.driver = {
- .name = "ds1553",
+ .name = "rtc-ds1553",
.owner = THIS_MODULE,
},
};
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: Make rtc-ds1742 driver hotplug-aware
On Fri, 17 Aug 2007 18:32:19 +0200, "Kay Sievers" <[EMAIL PROTECTED]> wrote: > > I guess there are some out-of-tree users of this driver, but fixing > > them is really trivial, so I don't think this is a big compatibility > > problem. > > Again, the only sane solution is to provide MODALIAS="platform:" > from the platform bus, and adding the aliases to drivers who support > autoloading. Modalias strings are not free-text strings, they are > required to be prefixed by the subsystem. I guess your objection is to my usage of MODULE_ALIAS, right? I will send a patch to change the driver name string, because I did not use the different name intentionally, I just had not thought about hotplugging at all. I think there is no serious reason to stick with current driver name anyway. --- Atsushi Nemoto - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: Make rtc-ds1742 driver hotplug-aware
On Fri, 17 Aug 2007 18:32:19 +0200, Kay Sievers [EMAIL PROTECTED] wrote: I guess there are some out-of-tree users of this driver, but fixing them is really trivial, so I don't think this is a big compatibility problem. Again, the only sane solution is to provide MODALIAS=platform:name from the platform bus, and adding the aliases to drivers who support autoloading. Modalias strings are not free-text strings, they are required to be prefixed by the subsystem. I guess your objection is to my usage of MODULE_ALIAS, right? I will send a patch to change the driver name string, because I did not use the different name intentionally, I just had not thought about hotplugging at all. I think there is no serious reason to stick with current driver name anyway. --- Atsushi Nemoto - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: Make rtc-rs5c348 driver hotplug-aware (take 2)
The rtc-rs5c348 SPI driver name doesn't match its module name, which
prevents it from properly hotplugging. There is only one in-tree user
of its driver, which is fixed by this patch too.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
arch/mips/tx4938/toshiba_rbtx4938/setup.c |2 +-
drivers/rtc/rtc-rs5c348.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/tx4938/toshiba_rbtx4938/setup.c
b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
index 57f3c70..84ebff7 100644
--- a/arch/mips/tx4938/toshiba_rbtx4938/setup.c
+++ b/arch/mips/tx4938/toshiba_rbtx4938/setup.c
@@ -1115,7 +1115,7 @@ static void __init txx9_spi_init(unsigned long base, int
irq)
static int __init rbtx4938_spi_init(void)
{
struct spi_board_info srtc_info = {
- .modalias = rs5c348,
+ .modalias = rtc-rs5c348,
.max_speed_hz = 100, /* 1.0Mbps @ Vdd 2.0V */
.bus_num = 0,
.chip_select = 16 + SRTC_CS,
diff --git a/drivers/rtc/rtc-rs5c348.c b/drivers/rtc/rtc-rs5c348.c
index f50f3fc..8394626 100644
--- a/drivers/rtc/rtc-rs5c348.c
+++ b/drivers/rtc/rtc-rs5c348.c
@@ -226,7 +226,7 @@ static int __devexit rs5c348_remove(struct spi_device *spi)
static struct spi_driver rs5c348_driver = {
.driver = {
- .name = rs5c348,
+ .name = rtc-rs5c348,
.bus= spi_bus_type,
.owner = THIS_MODULE,
},
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: Make rtc-ds1553 driver hotplug-aware (take 3)
The rtc-ds1553 platform driver name doesn't match its module name,
which might prevent it from properly hotplugging. This driver has no
in-tree users.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
The take 2 of this patch was just a typo-fix (1553 vs. 1742).
diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
index 46da571..a4bcb34 100644
--- a/drivers/rtc/rtc-ds1553.c
+++ b/drivers/rtc/rtc-ds1553.c
@@ -395,7 +395,7 @@ static struct platform_driver ds1553_rtc_driver = {
.probe = ds1553_rtc_probe,
.remove = __devexit_p(ds1553_rtc_remove),
.driver = {
- .name = ds1553,
+ .name = rtc-ds1553,
.owner = THIS_MODULE,
},
};
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: Make rtc-ds1742 driver hotplug-aware (take 2)
The rtc-ds1742 platform driver name doesn't match its module name,
which might prevents it from properly hotplugging. There is only two
in-tree user of its driver, which are fixed by this patch too.
Signed-off-by: Atsushi Nemoto [EMAIL PROTECTED]
---
arch/mips/jmr3927/rbhma3100/setup.c|2 +-
.../toshiba_rbtx4927/toshiba_rbtx4927_setup.c |2 +-
drivers/rtc/rtc-ds1742.c |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/jmr3927/rbhma3100/setup.c
b/arch/mips/jmr3927/rbhma3100/setup.c
index fde56e8..ffae163 100644
--- a/arch/mips/jmr3927/rbhma3100/setup.c
+++ b/arch/mips/jmr3927/rbhma3100/setup.c
@@ -427,7 +427,7 @@ static int __init jmr3927_rtc_init(void)
.flags = IORESOURCE_MEM,
};
struct platform_device *dev;
- dev = platform_device_register_simple(ds1742, -1, res, 1);
+ dev = platform_device_register_simple(rtc-ds1742, -1, res, 1);
return IS_ERR(dev) ? PTR_ERR(dev) : 0;
}
device_initcall(jmr3927_rtc_init);
diff --git a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
index 3e84237..91bd3d1 100644
--- a/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
+++ b/arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c
@@ -969,7 +969,7 @@ static int __init toshiba_rbtx4927_rtc_init(void)
.flags = IORESOURCE_MEM,
};
struct platform_device *dev =
- platform_device_register_simple(ds1742, -1, res, 1);
+ platform_device_register_simple(rtc-ds1742, -1, res, 1);
return IS_ERR(dev) ? PTR_ERR(dev) : 0;
}
device_initcall(toshiba_rbtx4927_rtc_init);
diff --git a/drivers/rtc/rtc-ds1742.c b/drivers/rtc/rtc-ds1742.c
index b2e5481..5562830 100644
--- a/drivers/rtc/rtc-ds1742.c
+++ b/drivers/rtc/rtc-ds1742.c
@@ -251,7 +251,7 @@ static struct platform_driver ds1742_rtc_driver = {
.probe = ds1742_rtc_probe,
.remove = __devexit_p(ds1742_rtc_remove),
.driver = {
- .name = ds1742,
+ .name = rtc-ds1742,
.owner = THIS_MODULE,
},
};
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: legacy platform drivers and hotplugging
On Sun, 19 Aug 2007 18:44:43 -0700, David Brownell [EMAIL PROTECTED] wrote: Notice I fixed $SUBJECT to not mention rtc-ds1742, which has never been a platform device and thus is offtopic to your specific complaints about platform bus hotplugging. Well, rtc-ds1742 *IS* a platform driver. But it seems the discussion is not DS1742 specific at all, and I guess ds1742 is not a good example for hotplugging, so I welcome your $SUBJECT fix :-) --- Atsushi Nemoto - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: Make rtc-ds1742 driver hotplug-aware
On Sun, 19 Aug 2007 19:02:04 -0700, David Brownell [EMAIL PROTECTED] wrote: This is indeed a platform driver. And v2 of this patch doesn't resolve its won't hotplug problem. The simplest way to resolve that would be switching to the more widely used platform_device_register(). Yes, if the current in-tree users (JMR3927 and RBTX4927 board) really wanted to hotplug this driver. And I believe they won't :-) I just want to fix a potential problem in rtc-ds1742 driver for someone who want to hotplug it. That's all. --- Atsushi Nemoto - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: Make rtc-ds1742 driver hotplug-aware
On Fri, 17 Aug 2007 09:02:00 -0700, David Brownell <[EMAIL PROTECTED]> wrote: > Because for some reason the driver name isn't rtc-ds1742 ... > > My preferred style for such patches puts the MODULE_ALIAS up > near the strange driver name, so it's more clear what's going > on. Putting all the MODULE_*() stuff at the end of the file > gets confusing in this case. OK, then I should update my patches anyway ;) Which do you prefer, MODULE_ALIAS or change some strings? I guess there are some out-of-tree users of this driver, but fixing them is really trivial, so I don't think this is a big compatibility problem. --- Atsushi Nemoto - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
