From: Vivek Goyal <vgoyal@in.ibm.com>
To: Greg KH <gregkh@suse.de>
Cc: Jeff Garzik <jeff@garzik.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH] 64bit resource: fix up printks for resources in ide drivers
Date: Thu, 29 Jun 2006 16:12:43 -0400 [thread overview]
Message-ID: <20060629201243.GA9945@in.ibm.com> (raw)
In-Reply-To: <20060629195110.GA3429@suse.de>
On Thu, Jun 29, 2006 at 12:51:10PM -0700, Greg KH wrote:
> On Thu, Jun 29, 2006 at 03:41:13PM -0400, Jeff Garzik wrote:
> > Linux Kernel Mailing List wrote:
> > >commit 08f46de9a0e7c293db34cf44f9451d18ef609770
> > >tree 83c28b79165adee350aad8cb9d4e2e59486acf56
> > >parent 176dfc633bbe4e03f4557d2beeefb4f0cc7f0efa
> > >author Greg Kroah-Hartman <gregkh@suse.de> Tue, 13 Jun 2006 05:15:59 -0700
> > >committer Greg Kroah-Hartman <gregkh@suse.de> Tue, 27 Jun 2006 23:23:59
> > >-0700
> > >
> > >[PATCH] 64bit resource: fix up printks for resources in ide drivers
> > >
> > >This is needed if we wish to change the size of the resource structures.
> > >
> > >Based on an original patch from Vivek Goyal <vgoyal@in.ibm.com>
> > >
> > >Cc: Vivek Goyal <vgoyal@in.ibm.com>
> > >Signed-off-by: Andrew Morton <akpm@osdl.org>
> > >Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> > >
> > > drivers/ide/pci/aec62xx.c | 3 ++-
> > > drivers/ide/pci/cmd64x.c | 3 ++-
> > > drivers/ide/pci/hpt34x.c | 2 +-
> > > drivers/ide/pci/pdc202xx_new.c | 4 ++--
> > > drivers/ide/pci/pdc202xx_old.c | 4 ++--
> > > 5 files changed, 9 insertions(+), 7 deletions(-)
> > >
> > >diff --git a/drivers/ide/pci/aec62xx.c b/drivers/ide/pci/aec62xx.c
> > >index c743e68..8d5b872 100644
> > >--- a/drivers/ide/pci/aec62xx.c
> > >+++ b/drivers/ide/pci/aec62xx.c
> > >@@ -254,7 +254,8 @@ static unsigned int __devinit init_chips
> > >
> > > if (dev->resource[PCI_ROM_RESOURCE].start) {
> > > pci_write_config_dword(dev, PCI_ROM_ADDRESS,
> > > dev->resource[PCI_ROM_RESOURCE].start |
> > > PCI_ROM_ADDRESS_ENABLE);
> > >- printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n", name,
> > >dev->resource[PCI_ROM_RESOURCE].start);
> > >+ printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n", name,
> > >+ (unsigned
> > >long)dev->resource[PCI_ROM_RESOURCE].start);
> > > }
> > >
> > > if (bus_speed <= 33)
> > >diff --git a/drivers/ide/pci/cmd64x.c b/drivers/ide/pci/cmd64x.c
> > >index 3d9c7af..9828039 100644
> > >--- a/drivers/ide/pci/cmd64x.c
> > >+++ b/drivers/ide/pci/cmd64x.c
> > >@@ -609,7 +609,8 @@ static unsigned int __devinit init_chips
> > > #ifdef __i386__
> > > if (dev->resource[PCI_ROM_RESOURCE].start) {
> > > pci_write_config_dword(dev, PCI_ROM_ADDRESS,
> > > dev->resource[PCI_ROM_RESOURCE].start |
> > > PCI_ROM_ADDRESS_ENABLE);
> > >- printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n", name,
> > >dev->resource[PCI_ROM_RESOURCE].start);
> > >+ printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n", name,
> > >+ (unsigned
> > >long)dev->resource[PCI_ROM_RESOURCE].start);
> > > }
> > > #endif
> > >
> > >diff --git a/drivers/ide/pci/hpt34x.c b/drivers/ide/pci/hpt34x.c
> > >index be334da..7da5502 100644
> > >--- a/drivers/ide/pci/hpt34x.c
> > >+++ b/drivers/ide/pci/hpt34x.c
> > >@@ -176,7 +176,7 @@ static unsigned int __devinit init_chips
> > > pci_write_config_dword(dev, PCI_ROM_ADDRESS,
> > > dev->resource[PCI_ROM_RESOURCE].start |
> > > PCI_ROM_ADDRESS_ENABLE);
> > > printk(KERN_INFO "HPT345: ROM enabled at 0x%08lx\n",
> > >- dev->resource[PCI_ROM_RESOURCE].start);
> > >+ (unsigned
> > >long)dev->resource[PCI_ROM_RESOURCE].start);
> > > }
> > > pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0xF0);
> > > } else {
> > >diff --git a/drivers/ide/pci/pdc202xx_new.c
> > >b/drivers/ide/pci/pdc202xx_new.c
> > >index acd6317..20d5965 100644
> > >--- a/drivers/ide/pci/pdc202xx_new.c
> > >+++ b/drivers/ide/pci/pdc202xx_new.c
> > >@@ -313,8 +313,8 @@ static unsigned int __devinit init_chips
> > > if (dev->resource[PCI_ROM_RESOURCE].start) {
> > > pci_write_config_dword(dev, PCI_ROM_ADDRESS,
> > > dev->resource[PCI_ROM_RESOURCE].start |
> > > PCI_ROM_ADDRESS_ENABLE);
> > >- printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n",
> > >- name, dev->resource[PCI_ROM_RESOURCE].start);
> > >+ printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n", name,
> > >+ (unsigned
> > >long)dev->resource[PCI_ROM_RESOURCE].start);
> > > }
> > >
> > > #ifdef CONFIG_PPC_PMAC
> > >diff --git a/drivers/ide/pci/pdc202xx_old.c
> > >b/drivers/ide/pci/pdc202xx_old.c
> > >index 22d1754..ffbef74 100644
> > >--- a/drivers/ide/pci/pdc202xx_old.c
> > >+++ b/drivers/ide/pci/pdc202xx_old.c
> > >@@ -544,8 +544,8 @@ static unsigned int __devinit init_chips
> > > if (dev->resource[PCI_ROM_RESOURCE].start) {
> > > pci_write_config_dword(dev, PCI_ROM_ADDRESS,
> > > dev->resource[PCI_ROM_RESOURCE].start |
> > > PCI_ROM_ADDRESS_ENABLE);
> > >- printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n",
> > >- name, dev->resource[PCI_ROM_RESOURCE].start);
> > >+ printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n", name,
> > >+ (unsigned
> > >long)dev->resource[PCI_ROM_RESOURCE].start);
> >
> > Why cast to unsigned long here? Won't that truncate the data in certain
> > cases, now that it is 64bit?
> >
> > Other printk patches seem to use unsigned long long, as I would expect.
>
> Yes it will truncate stuff. Vivek, any reason you did it this way for
> the ide drivers?
>
Initially I had kept it unsigned long long but later changed to unsigned long
because of following response from Alan Cox.
[start]
> pci_write_config_dword(dev, PCI_ROM_ADDRESS,
dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE);
> - printk(KERN_INFO "%s: ROM enabled at 0x%08lx\n", name,
dev->resource[PCI_ROM_RESOURCE].start);
> + printk(KERN_INFO "%s: ROM enabled at 0x%016llx\n", name,
> + (unsigned long
long)dev->resource[PCI_ROM_RESOURCE].start);
NAK - if the resource is 64bit then the pci_write_config_dword is also
insufficient. Ditto for each other example.
We actually know the PCI resources for these are 32bit so this change
shouldn't be needed. You might want to stick a (u32) or (unsigned long)
cast in and leave it at that.
As far as I can tell all the ROM whacking code is bogus anyway
[end]
Thanks
Vivek
prev parent reply other threads:[~2006-06-29 20:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <200606291800.k5TI0qfD002870@hera.kernel.org>
2006-06-29 19:41 ` [PATCH] 64bit resource: fix up printks for resources in ide drivers Jeff Garzik
2006-06-29 19:51 ` Greg KH
2006-06-29 20:12 ` Vivek Goyal [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060629201243.GA9945@in.ibm.com \
--to=vgoyal@in.ibm.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=gregkh@suse.de \
--cc=jeff@garzik.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.