| Message ID | 20201027171130.56998-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] xsysace: use platform_get_resource() and platform_get_irq_optional() | expand |
Hi Andy, On 27. 10. 20 18:11, Andy Shevchenko wrote: > Use platform_get_resource() to fetch the memory resource and > platform_get_irq_optional() to get optional IRQ instead of > open-coded variants. > > IRQ is not supposed to be changed at runtime, so there is > no functional change in ace_fsm_yieldirq(). > > On the other hand we now take first resources instead of last ones > to proceed. I can't imagine how broken should be firmware to have > a garbage in the first resource slots. But if it the case, it needs > to be documented. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/block/xsysace.c | 49 ++++++++++++++++++++++------------------- > 1 file changed, 26 insertions(+), 23 deletions(-) > > diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c > index 8d581c7536fb..eb8ef65778c3 100644 > --- a/drivers/block/xsysace.c > +++ b/drivers/block/xsysace.c > @@ -443,22 +443,27 @@ static void ace_fix_driveid(u16 *id) > #define ACE_FSM_NUM_STATES 11 > > /* Set flag to exit FSM loop and reschedule tasklet */ > -static inline void ace_fsm_yield(struct ace_device *ace) > +static inline void ace_fsm_yieldpoll(struct ace_device *ace) > { > - dev_dbg(ace->dev, "ace_fsm_yield()\n"); > tasklet_schedule(&ace->fsm_tasklet); > ace->fsm_continue_flag = 0; > } > > +static inline void ace_fsm_yield(struct ace_device *ace) > +{ > + dev_dbg(ace->dev, "%s()\n", __func__); > + ace_fsm_yieldpoll(ace); > +} > + > /* Set flag to exit FSM loop and wait for IRQ to reschedule tasklet */ > static inline void ace_fsm_yieldirq(struct ace_device *ace) > { > dev_dbg(ace->dev, "ace_fsm_yieldirq()\n"); > > - if (!ace->irq) > - /* No IRQ assigned, so need to poll */ > - tasklet_schedule(&ace->fsm_tasklet); > - ace->fsm_continue_flag = 0; > + if (ace->irq > 0) > + ace->fsm_continue_flag = 0; > + else > + ace_fsm_yieldpoll(ace); > } > > static bool ace_has_next_request(struct request_queue *q) > @@ -1053,12 +1058,12 @@ static int ace_setup(struct ace_device *ace) > ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ); > > /* Now we can hook up the irq handler */ > - if (ace->irq) { > + if (ace->irq > 0) { > rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace); > if (rc) { > /* Failure - fall back to polled mode */ > dev_err(ace->dev, "request_irq failed\n"); > - ace->irq = 0; > + ace->irq = rc; > } > } > > @@ -1110,7 +1115,7 @@ static void ace_teardown(struct ace_device *ace) > > tasklet_kill(&ace->fsm_tasklet); > > - if (ace->irq) > + if (ace->irq > 0) > free_irq(ace->irq, ace); > > iounmap(ace->baseaddr); > @@ -1123,11 +1128,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, > int rc; > dev_dbg(dev, "ace_alloc(%p)\n", dev); > > - if (!physaddr) { > - rc = -ENODEV; > - goto err_noreg; > - } > - > /* Allocate and initialize the ace device structure */ > ace = kzalloc(sizeof(struct ace_device), GFP_KERNEL); > if (!ace) { > @@ -1153,7 +1153,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, > dev_set_drvdata(dev, NULL); > kfree(ace); > err_alloc: > -err_noreg: > dev_err(dev, "could not initialize device, err=%i\n", rc); > return rc; > } > @@ -1176,10 +1175,11 @@ static void ace_free(struct device *dev) > > static int ace_probe(struct platform_device *dev) > { > - resource_size_t physaddr = 0; > int bus_width = ACE_BUS_WIDTH_16; /* FIXME: should not be hard coded */ > + resource_size_t physaddr; > + struct resource *res; > u32 id = dev->id; > - int irq = 0; > + int irq; > int i; > > dev_dbg(&dev->dev, "ace_probe(%p)\n", dev); > @@ -1190,12 +1190,15 @@ static int ace_probe(struct platform_device *dev) > if (of_find_property(dev->dev.of_node, "8-bit", NULL)) > bus_width = ACE_BUS_WIDTH_8; > > - for (i = 0; i < dev->num_resources; i++) { > - if (dev->resource[i].flags & IORESOURCE_MEM) > - physaddr = dev->resource[i].start; > - if (dev->resource[i].flags & IORESOURCE_IRQ) > - irq = dev->resource[i].start; > - } > + res = platform_get_resource(dev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > + > + physaddr = res->start; > + if (!physaddr) > + return -ENODEV; > + > + irq = platform_get_irq_optional(dev, 0); > > /* Call the bus-independent setup code */ > return ace_alloc(&dev->dev, id, physaddr, irq, bus_width); > This driver is quite old and obsolete. I am fine with whatever patch and I am also fine with marking driver as BROKEN or remove it because I am not aware about any user. Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
On 10/29/20 1:18 AM, Michal Simek wrote: > Hi Andy, > > On 27. 10. 20 18:11, Andy Shevchenko wrote: >> Use platform_get_resource() to fetch the memory resource and >> platform_get_irq_optional() to get optional IRQ instead of >> open-coded variants. >> >> IRQ is not supposed to be changed at runtime, so there is >> no functional change in ace_fsm_yieldirq(). >> >> On the other hand we now take first resources instead of last ones >> to proceed. I can't imagine how broken should be firmware to have >> a garbage in the first resource slots. But if it the case, it needs >> to be documented. >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> drivers/block/xsysace.c | 49 ++++++++++++++++++++++------------------- >> 1 file changed, 26 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c >> index 8d581c7536fb..eb8ef65778c3 100644 >> --- a/drivers/block/xsysace.c >> +++ b/drivers/block/xsysace.c >> @@ -443,22 +443,27 @@ static void ace_fix_driveid(u16 *id) >> #define ACE_FSM_NUM_STATES 11 >> >> /* Set flag to exit FSM loop and reschedule tasklet */ >> -static inline void ace_fsm_yield(struct ace_device *ace) >> +static inline void ace_fsm_yieldpoll(struct ace_device *ace) >> { >> - dev_dbg(ace->dev, "ace_fsm_yield()\n"); >> tasklet_schedule(&ace->fsm_tasklet); >> ace->fsm_continue_flag = 0; >> } >> >> +static inline void ace_fsm_yield(struct ace_device *ace) >> +{ >> + dev_dbg(ace->dev, "%s()\n", __func__); >> + ace_fsm_yieldpoll(ace); >> +} >> + >> /* Set flag to exit FSM loop and wait for IRQ to reschedule tasklet */ >> static inline void ace_fsm_yieldirq(struct ace_device *ace) >> { >> dev_dbg(ace->dev, "ace_fsm_yieldirq()\n"); >> >> - if (!ace->irq) >> - /* No IRQ assigned, so need to poll */ >> - tasklet_schedule(&ace->fsm_tasklet); >> - ace->fsm_continue_flag = 0; >> + if (ace->irq > 0) >> + ace->fsm_continue_flag = 0; >> + else >> + ace_fsm_yieldpoll(ace); >> } >> >> static bool ace_has_next_request(struct request_queue *q) >> @@ -1053,12 +1058,12 @@ static int ace_setup(struct ace_device *ace) >> ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ); >> >> /* Now we can hook up the irq handler */ >> - if (ace->irq) { >> + if (ace->irq > 0) { >> rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace); >> if (rc) { >> /* Failure - fall back to polled mode */ >> dev_err(ace->dev, "request_irq failed\n"); >> - ace->irq = 0; >> + ace->irq = rc; >> } >> } >> >> @@ -1110,7 +1115,7 @@ static void ace_teardown(struct ace_device *ace) >> >> tasklet_kill(&ace->fsm_tasklet); >> >> - if (ace->irq) >> + if (ace->irq > 0) >> free_irq(ace->irq, ace); >> >> iounmap(ace->baseaddr); >> @@ -1123,11 +1128,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, >> int rc; >> dev_dbg(dev, "ace_alloc(%p)\n", dev); >> >> - if (!physaddr) { >> - rc = -ENODEV; >> - goto err_noreg; >> - } >> - >> /* Allocate and initialize the ace device structure */ >> ace = kzalloc(sizeof(struct ace_device), GFP_KERNEL); >> if (!ace) { >> @@ -1153,7 +1153,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, >> dev_set_drvdata(dev, NULL); >> kfree(ace); >> err_alloc: >> -err_noreg: >> dev_err(dev, "could not initialize device, err=%i\n", rc); >> return rc; >> } >> @@ -1176,10 +1175,11 @@ static void ace_free(struct device *dev) >> >> static int ace_probe(struct platform_device *dev) >> { >> - resource_size_t physaddr = 0; >> int bus_width = ACE_BUS_WIDTH_16; /* FIXME: should not be hard coded */ >> + resource_size_t physaddr; >> + struct resource *res; >> u32 id = dev->id; >> - int irq = 0; >> + int irq; >> int i; >> >> dev_dbg(&dev->dev, "ace_probe(%p)\n", dev); >> @@ -1190,12 +1190,15 @@ static int ace_probe(struct platform_device *dev) >> if (of_find_property(dev->dev.of_node, "8-bit", NULL)) >> bus_width = ACE_BUS_WIDTH_8; >> >> - for (i = 0; i < dev->num_resources; i++) { >> - if (dev->resource[i].flags & IORESOURCE_MEM) >> - physaddr = dev->resource[i].start; >> - if (dev->resource[i].flags & IORESOURCE_IRQ) >> - irq = dev->resource[i].start; >> - } >> + res = platform_get_resource(dev, IORESOURCE_MEM, 0); >> + if (!res) >> + return -EINVAL; >> + >> + physaddr = res->start; >> + if (!physaddr) >> + return -ENODEV; >> + >> + irq = platform_get_irq_optional(dev, 0); >> >> /* Call the bus-independent setup code */ >> return ace_alloc(&dev->dev, id, physaddr, irq, bus_width); >> > > This driver is quite old and obsolete. I am fine with whatever patch and > I am also fine with marking driver as BROKEN or remove it because I am > not aware about any user. > > Acked-by: Michal Simek <michal.simek@xilinx.com> How about I queue this one up for 5.10, and then we can queue a removal patch for 5.11? Or at least schedule it for removal.
On 29. 10. 20 15:22, Jens Axboe wrote: > On 10/29/20 1:18 AM, Michal Simek wrote: >> Hi Andy, >> >> On 27. 10. 20 18:11, Andy Shevchenko wrote: >>> Use platform_get_resource() to fetch the memory resource and >>> platform_get_irq_optional() to get optional IRQ instead of >>> open-coded variants. >>> >>> IRQ is not supposed to be changed at runtime, so there is >>> no functional change in ace_fsm_yieldirq(). >>> >>> On the other hand we now take first resources instead of last ones >>> to proceed. I can't imagine how broken should be firmware to have >>> a garbage in the first resource slots. But if it the case, it needs >>> to be documented. >>> >>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> --- >>> drivers/block/xsysace.c | 49 ++++++++++++++++++++++------------------- >>> 1 file changed, 26 insertions(+), 23 deletions(-) >>> >>> diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c >>> index 8d581c7536fb..eb8ef65778c3 100644 >>> --- a/drivers/block/xsysace.c >>> +++ b/drivers/block/xsysace.c >>> @@ -443,22 +443,27 @@ static void ace_fix_driveid(u16 *id) >>> #define ACE_FSM_NUM_STATES 11 >>> >>> /* Set flag to exit FSM loop and reschedule tasklet */ >>> -static inline void ace_fsm_yield(struct ace_device *ace) >>> +static inline void ace_fsm_yieldpoll(struct ace_device *ace) >>> { >>> - dev_dbg(ace->dev, "ace_fsm_yield()\n"); >>> tasklet_schedule(&ace->fsm_tasklet); >>> ace->fsm_continue_flag = 0; >>> } >>> >>> +static inline void ace_fsm_yield(struct ace_device *ace) >>> +{ >>> + dev_dbg(ace->dev, "%s()\n", __func__); >>> + ace_fsm_yieldpoll(ace); >>> +} >>> + >>> /* Set flag to exit FSM loop and wait for IRQ to reschedule tasklet */ >>> static inline void ace_fsm_yieldirq(struct ace_device *ace) >>> { >>> dev_dbg(ace->dev, "ace_fsm_yieldirq()\n"); >>> >>> - if (!ace->irq) >>> - /* No IRQ assigned, so need to poll */ >>> - tasklet_schedule(&ace->fsm_tasklet); >>> - ace->fsm_continue_flag = 0; >>> + if (ace->irq > 0) >>> + ace->fsm_continue_flag = 0; >>> + else >>> + ace_fsm_yieldpoll(ace); >>> } >>> >>> static bool ace_has_next_request(struct request_queue *q) >>> @@ -1053,12 +1058,12 @@ static int ace_setup(struct ace_device *ace) >>> ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ); >>> >>> /* Now we can hook up the irq handler */ >>> - if (ace->irq) { >>> + if (ace->irq > 0) { >>> rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace); >>> if (rc) { >>> /* Failure - fall back to polled mode */ >>> dev_err(ace->dev, "request_irq failed\n"); >>> - ace->irq = 0; >>> + ace->irq = rc; >>> } >>> } >>> >>> @@ -1110,7 +1115,7 @@ static void ace_teardown(struct ace_device *ace) >>> >>> tasklet_kill(&ace->fsm_tasklet); >>> >>> - if (ace->irq) >>> + if (ace->irq > 0) >>> free_irq(ace->irq, ace); >>> >>> iounmap(ace->baseaddr); >>> @@ -1123,11 +1128,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, >>> int rc; >>> dev_dbg(dev, "ace_alloc(%p)\n", dev); >>> >>> - if (!physaddr) { >>> - rc = -ENODEV; >>> - goto err_noreg; >>> - } >>> - >>> /* Allocate and initialize the ace device structure */ >>> ace = kzalloc(sizeof(struct ace_device), GFP_KERNEL); >>> if (!ace) { >>> @@ -1153,7 +1153,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, >>> dev_set_drvdata(dev, NULL); >>> kfree(ace); >>> err_alloc: >>> -err_noreg: >>> dev_err(dev, "could not initialize device, err=%i\n", rc); >>> return rc; >>> } >>> @@ -1176,10 +1175,11 @@ static void ace_free(struct device *dev) >>> >>> static int ace_probe(struct platform_device *dev) >>> { >>> - resource_size_t physaddr = 0; >>> int bus_width = ACE_BUS_WIDTH_16; /* FIXME: should not be hard coded */ >>> + resource_size_t physaddr; >>> + struct resource *res; >>> u32 id = dev->id; >>> - int irq = 0; >>> + int irq; >>> int i; >>> >>> dev_dbg(&dev->dev, "ace_probe(%p)\n", dev); >>> @@ -1190,12 +1190,15 @@ static int ace_probe(struct platform_device *dev) >>> if (of_find_property(dev->dev.of_node, "8-bit", NULL)) >>> bus_width = ACE_BUS_WIDTH_8; >>> >>> - for (i = 0; i < dev->num_resources; i++) { >>> - if (dev->resource[i].flags & IORESOURCE_MEM) >>> - physaddr = dev->resource[i].start; >>> - if (dev->resource[i].flags & IORESOURCE_IRQ) >>> - irq = dev->resource[i].start; >>> - } >>> + res = platform_get_resource(dev, IORESOURCE_MEM, 0); >>> + if (!res) >>> + return -EINVAL; >>> + >>> + physaddr = res->start; >>> + if (!physaddr) >>> + return -ENODEV; >>> + >>> + irq = platform_get_irq_optional(dev, 0); >>> >>> /* Call the bus-independent setup code */ >>> return ace_alloc(&dev->dev, id, physaddr, irq, bus_width); >>> >> >> This driver is quite old and obsolete. I am fine with whatever patch and >> I am also fine with marking driver as BROKEN or remove it because I am >> not aware about any user. >> >> Acked-by: Michal Simek <michal.simek@xilinx.com> > > How about I queue this one up for 5.10, and then we can queue a removal > patch for 5.11? Or at least schedule it for removal. > Works for me. I will send removal patch for 5.11. Thanks, Michal
On 30. 10. 20 8:08, Michal Simek wrote: > > > On 29. 10. 20 15:22, Jens Axboe wrote: >> On 10/29/20 1:18 AM, Michal Simek wrote: >>> Hi Andy, >>> >>> On 27. 10. 20 18:11, Andy Shevchenko wrote: >>>> Use platform_get_resource() to fetch the memory resource and >>>> platform_get_irq_optional() to get optional IRQ instead of >>>> open-coded variants. >>>> >>>> IRQ is not supposed to be changed at runtime, so there is >>>> no functional change in ace_fsm_yieldirq(). >>>> >>>> On the other hand we now take first resources instead of last ones >>>> to proceed. I can't imagine how broken should be firmware to have >>>> a garbage in the first resource slots. But if it the case, it needs >>>> to be documented. >>>> >>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>>> --- >>>> drivers/block/xsysace.c | 49 ++++++++++++++++++++++------------------- >>>> 1 file changed, 26 insertions(+), 23 deletions(-) >>>> >>>> diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c >>>> index 8d581c7536fb..eb8ef65778c3 100644 >>>> --- a/drivers/block/xsysace.c >>>> +++ b/drivers/block/xsysace.c >>>> @@ -443,22 +443,27 @@ static void ace_fix_driveid(u16 *id) >>>> #define ACE_FSM_NUM_STATES 11 >>>> >>>> /* Set flag to exit FSM loop and reschedule tasklet */ >>>> -static inline void ace_fsm_yield(struct ace_device *ace) >>>> +static inline void ace_fsm_yieldpoll(struct ace_device *ace) >>>> { >>>> - dev_dbg(ace->dev, "ace_fsm_yield()\n"); >>>> tasklet_schedule(&ace->fsm_tasklet); >>>> ace->fsm_continue_flag = 0; >>>> } >>>> >>>> +static inline void ace_fsm_yield(struct ace_device *ace) >>>> +{ >>>> + dev_dbg(ace->dev, "%s()\n", __func__); >>>> + ace_fsm_yieldpoll(ace); >>>> +} >>>> + >>>> /* Set flag to exit FSM loop and wait for IRQ to reschedule tasklet */ >>>> static inline void ace_fsm_yieldirq(struct ace_device *ace) >>>> { >>>> dev_dbg(ace->dev, "ace_fsm_yieldirq()\n"); >>>> >>>> - if (!ace->irq) >>>> - /* No IRQ assigned, so need to poll */ >>>> - tasklet_schedule(&ace->fsm_tasklet); >>>> - ace->fsm_continue_flag = 0; >>>> + if (ace->irq > 0) >>>> + ace->fsm_continue_flag = 0; >>>> + else >>>> + ace_fsm_yieldpoll(ace); >>>> } >>>> >>>> static bool ace_has_next_request(struct request_queue *q) >>>> @@ -1053,12 +1058,12 @@ static int ace_setup(struct ace_device *ace) >>>> ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ); >>>> >>>> /* Now we can hook up the irq handler */ >>>> - if (ace->irq) { >>>> + if (ace->irq > 0) { >>>> rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace); >>>> if (rc) { >>>> /* Failure - fall back to polled mode */ >>>> dev_err(ace->dev, "request_irq failed\n"); >>>> - ace->irq = 0; >>>> + ace->irq = rc; >>>> } >>>> } >>>> >>>> @@ -1110,7 +1115,7 @@ static void ace_teardown(struct ace_device *ace) >>>> >>>> tasklet_kill(&ace->fsm_tasklet); >>>> >>>> - if (ace->irq) >>>> + if (ace->irq > 0) >>>> free_irq(ace->irq, ace); >>>> >>>> iounmap(ace->baseaddr); >>>> @@ -1123,11 +1128,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, >>>> int rc; >>>> dev_dbg(dev, "ace_alloc(%p)\n", dev); >>>> >>>> - if (!physaddr) { >>>> - rc = -ENODEV; >>>> - goto err_noreg; >>>> - } >>>> - >>>> /* Allocate and initialize the ace device structure */ >>>> ace = kzalloc(sizeof(struct ace_device), GFP_KERNEL); >>>> if (!ace) { >>>> @@ -1153,7 +1153,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, >>>> dev_set_drvdata(dev, NULL); >>>> kfree(ace); >>>> err_alloc: >>>> -err_noreg: >>>> dev_err(dev, "could not initialize device, err=%i\n", rc); >>>> return rc; >>>> } >>>> @@ -1176,10 +1175,11 @@ static void ace_free(struct device *dev) >>>> >>>> static int ace_probe(struct platform_device *dev) >>>> { >>>> - resource_size_t physaddr = 0; >>>> int bus_width = ACE_BUS_WIDTH_16; /* FIXME: should not be hard coded */ >>>> + resource_size_t physaddr; >>>> + struct resource *res; >>>> u32 id = dev->id; >>>> - int irq = 0; >>>> + int irq; >>>> int i; >>>> >>>> dev_dbg(&dev->dev, "ace_probe(%p)\n", dev); >>>> @@ -1190,12 +1190,15 @@ static int ace_probe(struct platform_device *dev) >>>> if (of_find_property(dev->dev.of_node, "8-bit", NULL)) >>>> bus_width = ACE_BUS_WIDTH_8; >>>> >>>> - for (i = 0; i < dev->num_resources; i++) { >>>> - if (dev->resource[i].flags & IORESOURCE_MEM) >>>> - physaddr = dev->resource[i].start; >>>> - if (dev->resource[i].flags & IORESOURCE_IRQ) >>>> - irq = dev->resource[i].start; >>>> - } >>>> + res = platform_get_resource(dev, IORESOURCE_MEM, 0); >>>> + if (!res) >>>> + return -EINVAL; >>>> + >>>> + physaddr = res->start; >>>> + if (!physaddr) >>>> + return -ENODEV; >>>> + >>>> + irq = platform_get_irq_optional(dev, 0); >>>> >>>> /* Call the bus-independent setup code */ >>>> return ace_alloc(&dev->dev, id, physaddr, irq, bus_width); >>>> >>> >>> This driver is quite old and obsolete. I am fine with whatever patch and >>> I am also fine with marking driver as BROKEN or remove it because I am >>> not aware about any user. >>> >>> Acked-by: Michal Simek <michal.simek@xilinx.com> >> >> How about I queue this one up for 5.10, and then we can queue a removal >> patch for 5.11? Or at least schedule it for removal. >> > > Works for me. I will send removal patch for 5.11. patch sent. M
diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c index 8d581c7536fb..eb8ef65778c3 100644 --- a/drivers/block/xsysace.c +++ b/drivers/block/xsysace.c @@ -443,22 +443,27 @@ static void ace_fix_driveid(u16 *id) #define ACE_FSM_NUM_STATES 11 /* Set flag to exit FSM loop and reschedule tasklet */ -static inline void ace_fsm_yield(struct ace_device *ace) +static inline void ace_fsm_yieldpoll(struct ace_device *ace) { - dev_dbg(ace->dev, "ace_fsm_yield()\n"); tasklet_schedule(&ace->fsm_tasklet); ace->fsm_continue_flag = 0; } +static inline void ace_fsm_yield(struct ace_device *ace) +{ + dev_dbg(ace->dev, "%s()\n", __func__); + ace_fsm_yieldpoll(ace); +} + /* Set flag to exit FSM loop and wait for IRQ to reschedule tasklet */ static inline void ace_fsm_yieldirq(struct ace_device *ace) { dev_dbg(ace->dev, "ace_fsm_yieldirq()\n"); - if (!ace->irq) - /* No IRQ assigned, so need to poll */ - tasklet_schedule(&ace->fsm_tasklet); - ace->fsm_continue_flag = 0; + if (ace->irq > 0) + ace->fsm_continue_flag = 0; + else + ace_fsm_yieldpoll(ace); } static bool ace_has_next_request(struct request_queue *q) @@ -1053,12 +1058,12 @@ static int ace_setup(struct ace_device *ace) ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ); /* Now we can hook up the irq handler */ - if (ace->irq) { + if (ace->irq > 0) { rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace); if (rc) { /* Failure - fall back to polled mode */ dev_err(ace->dev, "request_irq failed\n"); - ace->irq = 0; + ace->irq = rc; } } @@ -1110,7 +1115,7 @@ static void ace_teardown(struct ace_device *ace) tasklet_kill(&ace->fsm_tasklet); - if (ace->irq) + if (ace->irq > 0) free_irq(ace->irq, ace); iounmap(ace->baseaddr); @@ -1123,11 +1128,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, int rc; dev_dbg(dev, "ace_alloc(%p)\n", dev); - if (!physaddr) { - rc = -ENODEV; - goto err_noreg; - } - /* Allocate and initialize the ace device structure */ ace = kzalloc(sizeof(struct ace_device), GFP_KERNEL); if (!ace) { @@ -1153,7 +1153,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr, dev_set_drvdata(dev, NULL); kfree(ace); err_alloc: -err_noreg: dev_err(dev, "could not initialize device, err=%i\n", rc); return rc; } @@ -1176,10 +1175,11 @@ static void ace_free(struct device *dev) static int ace_probe(struct platform_device *dev) { - resource_size_t physaddr = 0; int bus_width = ACE_BUS_WIDTH_16; /* FIXME: should not be hard coded */ + resource_size_t physaddr; + struct resource *res; u32 id = dev->id; - int irq = 0; + int irq; int i; dev_dbg(&dev->dev, "ace_probe(%p)\n", dev); @@ -1190,12 +1190,15 @@ static int ace_probe(struct platform_device *dev) if (of_find_property(dev->dev.of_node, "8-bit", NULL)) bus_width = ACE_BUS_WIDTH_8; - for (i = 0; i < dev->num_resources; i++) { - if (dev->resource[i].flags & IORESOURCE_MEM) - physaddr = dev->resource[i].start; - if (dev->resource[i].flags & IORESOURCE_IRQ) - irq = dev->resource[i].start; - } + res = platform_get_resource(dev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + + physaddr = res->start; + if (!physaddr) + return -ENODEV; + + irq = platform_get_irq_optional(dev, 0); /* Call the bus-independent setup code */ return ace_alloc(&dev->dev, id, physaddr, irq, bus_width);
Use platform_get_resource() to fetch the memory resource and platform_get_irq_optional() to get optional IRQ instead of open-coded variants. IRQ is not supposed to be changed at runtime, so there is no functional change in ace_fsm_yieldirq(). On the other hand we now take first resources instead of last ones to proceed. I can't imagine how broken should be firmware to have a garbage in the first resource slots. But if it the case, it needs to be documented. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/block/xsysace.c | 49 ++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 23 deletions(-)