Re: [PATCH] /drivers/char sx.c ioremap -> pci_ioremap api

Re: [PATCH] /drivers/char sx.c ioremap -> pci_ioremap api

[Scott Thompson]

On Fri, 24 Aug 2007 15:15:30 -0400 Alan Cox 
<alan@lxorguk.ukuu.org.uk> wrote:
>ase, 0x80);
>> 
>> remove also the hwbase var.
>> 
>> > +	rebase = pci_iomap(pdev, 0, 0x80);
>> >   	t = readl(rebase + CNTRL_REG_OFFSET);
>
>Switch to ioread* if you are using the iomap interface. Its not a 
>trivial
>conversion and its slower and bulkier - the original ioremap was 
>much
>better
>
>NAK this change therefore

Jiri had requested this in relation to previous patch on unchecked 
ioremap return codes, but if the original ioremap is better NAK is 
fine
here and I won't resubmit w/ the hwbase var pulled out or the readl 
-> ioread32 switchover.

---------------------------------------
Scott Thompson / postfail@hushmail.com
---------------------------------------

Re: [PATCH] /drivers/char sx.c ioremap -> pci_ioremap api

[Alan Cox]

On Fri, 24 Aug 2007 22:03:42 +0200
Jiri Slaby <jirislaby@gmail.com> wrote:

> Alan Cox napsal(a):
> > ase, 0x80);
> >> remove also the hwbase var.
> >>
> >>> +	rebase = pci_iomap(pdev, 0, 0x80);
> >>>   	t = readl(rebase + CNTRL_REG_OFFSET);
> > 
> > Switch to ioread* if you are using the iomap interface. Its not a trivial
> 
> Why, if you know it's surely a mem region (and thus you rely on it and do
> ioremap)? There are many places in the kernel, where this approach is used, e.g.
> libata piix.

Every single one of them is wrong. The encoding of iomap values is
platform dependant. Fixed in my tree.
> 
> > conversion and its slower and bulkier - the original ioremap was much
> > better
> 
> at least get rid of the reading the hwbase address from pci conf space, use
> pci_resource_start instead.

Agreed entirely

Re: [PATCH] /drivers/char sx.c ioremap -> pci_ioremap api

[Jiri Slaby]

Alan Cox napsal(a):
> ase, 0x80);
>> remove also the hwbase var.
>>
>>> +	rebase = pci_iomap(pdev, 0, 0x80);
>>>   	t = readl(rebase + CNTRL_REG_OFFSET);
> 
> Switch to ioread* if you are using the iomap interface. Its not a trivial

Why, if you know it's surely a mem region (and thus you rely on it and do
ioremap)? There are many places in the kernel, where this approach is used, e.g.
libata piix.

> conversion and its slower and bulkier - the original ioremap was much
> better

at least get rid of the reading the hwbase address from pci conf space, use
pci_resource_start instead.

-- 
Jiri Slaby (jirislaby@gmail.com)
Faculty of Informatics, Masaryk University

Re: [PATCH] /drivers/char sx.c ioremap -> pci_ioremap api

[Alan Cox]

ase, 0x80);
> 
> remove also the hwbase var.
> 
> > +	rebase = pci_iomap(pdev, 0, 0x80);
> >   	t = readl(rebase + CNTRL_REG_OFFSET);

Switch to ioread* if you are using the iomap interface. Its not a trivial
conversion and its slower and bulkier - the original ioremap was much
better

NAK this change therefore

Re: [PATCH] /drivers/char sx.c ioremap -> pci_ioremap api

[Jiri Slaby]

On 8/24/07, Scott Thompson <postfailathushmail@gmail.com> wrote:
>
> patchset against 2.6.23-rc3.
>
> moves the iomap/iounmap call to pci 'flavor'.  this was a
> recommendation from a previously submitted patch that handles
> the unchecked iomap (or, now, pci_iomap) return code.
>
> Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> ----------------------------------------------------------
>
> diff --git a/drivers/char/sx.c b/drivers/char/sx.c
> index 85a2328..481334f 100644
> --- a/drivers/char/sx.c
> +++ b/drivers/char/sx.c
> @@ -2626,14 +2626,14 @@ static void __devinit fix_sx_pci(struct pci_dev
> *pdev, struct sx_board *board)
>
>   	pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &hwbase);
>   	hwbase &= PCI_BASE_ADDRESS_MEM_MASK;
> -	rebase = ioremap(hwbase, 0x80);

remove also the hwbase var.

> +	rebase = pci_iomap(pdev, 0, 0x80);
>   	t = readl(rebase + CNTRL_REG_OFFSET);
>   	if (t != CNTRL_REG_GOODVALUE) {
>   		printk(KERN_DEBUG "sx: performing cntrl reg fix: %08x -> "
>   			"%08x\n", t, CNTRL_REG_GOODVALUE);
>   		writel(CNTRL_REG_GOODVALUE, rebase + CNTRL_REG_OFFSET);
>   	}
> -	iounmap(rebase);
> +	pci_iounmap(pdev, rebase);
>   }
>   #endif
>


-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

[PATCH] /drivers/char sx.c ioremap -> pci_ioremap api

[Scott Thompson]

patchset against 2.6.23-rc3.

moves the iomap/iounmap call to pci 'flavor'.  this was a
recommendation from a previously submitted patch that handles
the unchecked iomap (or, now, pci_iomap) return code.

Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
----------------------------------------------------------

diff --git a/drivers/char/sx.c b/drivers/char/sx.c
index 85a2328..481334f 100644
--- a/drivers/char/sx.c
+++ b/drivers/char/sx.c
@@ -2626,14 +2626,14 @@ static void __devinit fix_sx_pci(struct pci_dev *pdev, struct sx_board *board)

  	pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &hwbase);
  	hwbase &= PCI_BASE_ADDRESS_MEM_MASK;
-	rebase = ioremap(hwbase, 0x80);
+	rebase = pci_iomap(pdev, 0, 0x80);
  	t = readl(rebase + CNTRL_REG_OFFSET);
  	if (t != CNTRL_REG_GOODVALUE) {
  		printk(KERN_DEBUG "sx: performing cntrl reg fix: %08x -> "
  			"%08x\n", t, CNTRL_REG_GOODVALUE);
  		writel(CNTRL_REG_GOODVALUE, rebase + CNTRL_REG_OFFSET);
  	}
-	iounmap(rebase);
+	pci_iounmap(pdev, rebase);
  }
  #endif