On Monday 11 January 2010 12:24:58 Lothar Waßmann wrote:
> Greg KH writes:
> > On Sun, Jan 10, 2010 at 03:44:09AM +0000, Ben Hutchings wrote:
> > > This code won't compile on ARM, on anything after 2.6.26:
> > >
> > > #ifdef CONFIG_ARM
> > > if (isp1362_hcd->board)
> > > set_irq_type(irq, isp1362_hcd->board->int_act_high ?
> > > IRQT_RISING : IRQT_FALLING); #endif
> > >
> > > Which is rather odd for a driver primarily meant for ARM systems and
> > > merged in 2.6.32...
too bad no one on the ARM side had a vested interest in the footwork required
to get it merged ... and no one noticed in the process since there were no ARM
consumers
> > > (Maybe this one should have gone through staging, Greg?)
that isnt really what staging is for
> The following patch should fix this, but the currently only users of
> the driver according to the defconfigs (blackfin bf527, bf537) all
> have IORESOURCE_IRQ_HIGHLEVEL in their IRQ_RESOURCE flags, while the
> driver was passing IRQF_TRIGGER_LOW to the usb_add_hcd() function. So
> either the setting in the driver or in the platform code was
> incorrect.
>
> I added the blackfin maintainer to the 'CC' list for commenting on
> this.
thanks, i'll raise an item on our side to look into this
> - dev_info(hcd->self.controller, " INTL: %4d * (%3lu+8): %4d @ $%04x\n",
> + dev_info(hcd->self.controller, " INTL: %4d * (%3u+8): %4d @ $%04x\n",
> ISP1362_INTL_BUFFERS, intl_blksize - PTD_HEADER_SIZE,
> intl_size, istl_size);
> - dev_info(hcd->self.controller, " ATL : %4d * (%3lu+8): %4d @ $%04x\n",
> + dev_info(hcd->self.controller, " ATL : %4d * (%3u+8): %4d @ $%04x\n",
> atl_buffers, atl_blksize - PTD_HEADER_SIZE,
> atl_size, istl_size + intl_size);
this merely shifts the warning around from some people to others, as my change
from %u to %lu did. i'm pretty sure we want to use %zu here and not %lu or
%u. there has been other posts on the list about this.
> @@ -2711,6 +2711,8 @@
> void __iomem *data_reg;
> int irq;
> int retval = 0;
> + struct resource *irq_res;
> + unsigned int irq_flags = 0;
>
> /* basic sanity checks first. board-specific init logic should
> * have initialized this the three resources and probably board
> @@ -2724,11 +2726,12 @@
>
> data = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> addr = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - irq = platform_get_irq(pdev, 0);
> - if (!addr || !data || irq < 0) {
> + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!addr || !data || irq_res) {
> retval = -ENODEV;
> goto err1;
> }
> + irq = irq_res->start;
what's wrong with the existing platform_get_irq() ? common code already
returns an error (-ENXIO) if the IRQ resource doesnt exist.
or did you just forget to initialize irq_flags here ? the new code sets
irq_flags to 0 and nothing else ...
-mike
Attachment:
signature.asc
Description: This is a digitally signed message part.
