Re: [PATCH 08/12] Drivers: hv: vmbus: Implement Direct Mode for stimer0

[Dan Carpenter]

On Wed, Feb 14, 2018 at 02:58:41AM +, Michael Kelley (EOSG) wrote:
> > -Original Message-
> > From: Dan Carpenter 
> > Sent: Monday, February 12, 2018 12:42 AM
> > To: KY Srinivasan ; Stephen Hemminger
> > 
> > Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; 
> > de...@linuxdriverproject.org;
> > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com; 
> > jasow...@redhat.com;
> > leann.ogasaw...@canonical.com; marcelo.ce...@canonical.com; Stephen 
> > Hemminger
> > ; Michael Kelley (EOSG) 
> > 
> > Subject: Re: [PATCH 08/12] Drivers: hv: vmbus: Implement Direct Mode for 
> > stimer0
> > 
> > On Sun, Feb 11, 2018 at 05:33:16PM -0700, k...@exchange.microsoft.com wrote:
> > > @@ -116,9 +146,29 @@ static int hv_ce_set_oneshot(struct 
> > > clock_event_device *evt)
> > >  {
> > >   union hv_timer_config timer_cfg;
> > >
> > > + timer_cfg.as_uint64 = 0;
> > >   timer_cfg.enable = 1;
> > >   timer_cfg.auto_enable = 1;
> > > - timer_cfg.sintx = VMBUS_MESSAGE_SINT;
> > > + if (direct_mode_enabled)
> > > + /*
> > > +  * When it expires, the timer will directly interrupt
> > > +  * on the specified hardware vector/IRQ.
> > > +  */
> > > + {
> > > + timer_cfg.direct_mode = 1;
> > > + timer_cfg.apic_vector = stimer0_vector;
> > > + hv_enable_stimer0_percpu_irq(stimer0_irq);
> > > + }
> > > + else
> > > + /*
> > > +  * When it expires, the timer will generate a VMbus message,
> > > +  * to be handled by the normal VMbus interrupt handler.
> > > +  */
> > > + {
> > > + timer_cfg.direct_mode = 0;
> > > + timer_cfg.sintx = VMBUS_MESSAGE_SINT;
> > > + }
> > > +
> > 
> > This indenting isn't right.  We should probably zero out .apic_vector
> > if .direct_mode is zero.  Or maybe it's fine.  I don't know if any
> > static analysis tools will complain...
> 
> I'll fix the indenting.  Old habits 
> 
> The " timer_cfg.as_uint64 = 0" statement already zero's out .apic_vector
> along with all the other unused fields in the 64-bit value, as required by
> the Hyper-V spec.

Ah, you're right, of course.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 08/12] Drivers: hv: vmbus: Implement Direct Mode for stimer0

[Michael Kelley (EOSG)]

> -Original Message-
> From: Dan Carpenter 
> Sent: Monday, February 12, 2018 12:42 AM
> To: KY Srinivasan ; Stephen Hemminger
> 
> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; 
> de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com; jasow...@redhat.com;
> leann.ogasaw...@canonical.com; marcelo.ce...@canonical.com; Stephen Hemminger
> ; Michael Kelley (EOSG) 
> 
> Subject: Re: [PATCH 08/12] Drivers: hv: vmbus: Implement Direct Mode for 
> stimer0
> 
> On Sun, Feb 11, 2018 at 05:33:16PM -0700, k...@exchange.microsoft.com wrote:
> > @@ -116,9 +146,29 @@ static int hv_ce_set_oneshot(struct clock_event_device 
> > *evt)
> >  {
> > union hv_timer_config timer_cfg;
> >
> > +   timer_cfg.as_uint64 = 0;
> > timer_cfg.enable = 1;
> > timer_cfg.auto_enable = 1;
> > -   timer_cfg.sintx = VMBUS_MESSAGE_SINT;
> > +   if (direct_mode_enabled)
> > +   /*
> > +* When it expires, the timer will directly interrupt
> > +* on the specified hardware vector/IRQ.
> > +*/
> > +   {
> > +   timer_cfg.direct_mode = 1;
> > +   timer_cfg.apic_vector = stimer0_vector;
> > +   hv_enable_stimer0_percpu_irq(stimer0_irq);
> > +   }
> > +   else
> > +   /*
> > +* When it expires, the timer will generate a VMbus message,
> > +* to be handled by the normal VMbus interrupt handler.
> > +*/
> > +   {
> > +   timer_cfg.direct_mode = 0;
> > +   timer_cfg.sintx = VMBUS_MESSAGE_SINT;
> > +   }
> > +
> 
> This indenting isn't right.  We should probably zero out .apic_vector
> if .direct_mode is zero.  Or maybe it's fine.  I don't know if any
> static analysis tools will complain...

I'll fix the indenting.  Old habits 

The " timer_cfg.as_uint64 = 0" statement already zero's out .apic_vector
along with all the other unused fields in the 64-bit value, as required by
the Hyper-V spec.

> 
> > hv_init_timer_config(HV_X64_MSR_STIMER0_CONFIG, timer_cfg.as_uint64);
> >
> > return 0;
> > @@ -191,6 +241,10 @@ int hv_synic_alloc(void)
> > INIT_LIST_HEAD(_cpu->chan_list);
> > }
> >
> > +   if (direct_mode_enabled && hv_setup_stimer0_irq(
> > +   _irq, _vector, hv_stimer0_isr))
> > +   goto err;
> 
> 
> Can you indent it like this:
> 
>   if (direct_mode_enabled &&
>   hv_setup_stimer0_irq(_irq, _vector,
>hv_stimer0_isr))
>   goto err;

OK -- will change.

> 
> 
> [ What follows is a long rant not directed at you ]
> 
> It's annoying because as soon as I see the "goto err;", I know the error
> handling for this function is going to be buggy...
> 
> Some rules of error handling are:
> 
> 1)  Each function should clean up after itself instead returning
> partially allocated structures.
> 2)  Each allocation function should have a matching free function.
> 3)  Give meaningful label names based on what the label location so that
> we can tell what the goto does just by looking at it, such as,
> "goto free_some_variable".  This way we can just keep a mental tally
> of the most recently allocated resource and verify based on the
> "goto free_resource;" statemetn that it frees the correct thing.  We
> don't need to scroll to the bottom of the function.
> 
> Using good names means that we should avoid do-nothing gotos
> because, by definition, the label name for a do-nothing goto is
> going to be vague.
> 
> In this case the label looks like this:
> 
> > +
> > return 0;
> >  err:
> > return -ENOMEM;
> 
> We allocate a bunch of stuff in this function so at first glance this
> looks like we leak everything but, actually, the cleanup is done in
> vmbus_bus_init().  This is a layering violation.
> 
> Later on, we changed hv_synic_alloc() in 37cdd991fac8 ("vmbus: put
> related per-cpu variable together") and we started allocating:
> 
>   hv_cpu->clk_evt = kzalloc(...
> 
> but we forgot to update the error handling because it was in the wrong
> place.  It's a very predictable, avoidable bug if we just use proper
> error handling style.

We'll fix this is a separate patch.  There are a couple other minor things
that should be cleaned up in hv.c as well, and can do them together.

Michael

> 
> Anyway...  Sorry for the long rant.  Summary:  Always distrust vague
> label names.
> 
> regards,
> dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH resend 1/3] staging: android: ion: Remove unused declaration ion_buffer_fault_user_mappings

[Yisheng Xie]

Hi Greg

On 2018/2/12 19:40, Yisheng Xie wrote:
> Hi Greg,
> 
> JFYI, I have rebase this patchset to v4.15-rc1.[1]

Ah, sorry , I mean v4.16-rc1, and the same to other threads.

Thanks
Yisheng

> 
> [1] https://lkml.org/lkml/2018/2/12/204
> 
> Thanks
> Yisheng
> 
> On 2018/2/1 9:54, Yisheng Xie wrote:
>> ion_buffer_fault_user_mappings's definition has been removed and not be
>> used anymore, just remove its useless declaration.
>>
>> Signed-off-by: Yisheng Xie 
>> ---
>>  drivers/staging/android/ion/ion.h | 9 -
>>  1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.h 
>> b/drivers/staging/android/ion/ion.h
>> index f5f9cd6..2160c35 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -201,15 +201,6 @@ struct ion_heap {
>>  bool ion_buffer_cached(struct ion_buffer *buffer);
>>  
>>  /**
>> - * ion_buffer_fault_user_mappings - fault in user mappings of this buffer
>> - * @buffer: buffer
>> - *
>> - * indicates whether userspace mappings of this buffer will be faulted
>> - * in, this can affect how buffers are allocated from the heap.
>> - */
>> -bool ion_buffer_fault_user_mappings(struct ion_buffer *buffer);
>> -
>> -/**
>>   * ion_device_add_heap - adds a heap to the ion device
>>   * @heap:   the heap to add
>>   */
>>
> 
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: android: ion: potential coherency issue

[Alexey Skidanov]

On 02/13/2018 08:40 PM, Laura Abbott wrote:
> On 02/12/2018 11:24 PM, Alexey Skidanov wrote:
>> Hello,
>>
>> Correct me if I'm wrong, but there is no user space interface, similar
>> to the dma_buf_start_cpu_access()/dma_buf_end_cpu_access() to handle IO
>> coherency. That is, on the platforms, where the SW is responsible for IO
>> coherency handling, the following sequences:
>> - write to the buffer
>> - DMA read
>> or
>> - DMA write
>> - read from the buffer
>> may be problematic.
>>
>> Thanks,
>> Alexey
>>
> 
> Have you looked at the dma_buf sync ioctl? (see
> include/uapi/linux/dma-buf.h)
> 
> Thanks,
> Laura

No ... But this is it :)

Thanks a lot,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH char-misc 1/1] Drivers: hv: vmbus: Fix ring buffer signaling

[Stephen Hemminger]

>   if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) {
>   u32 pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
>  
>   /*
> +  * Ensure the read of write_index in hv_get_bytes_to_write()
> +  * happens after the read of pending_send_sz.
> +  */
> + virt_rmb();
> + curr_write_sz = hv_get_bytes_to_write(rbi);
> +
> + /*
>* If there was space before we began iteration,
>* then host was not blocked. Also handles case where
>* pending_sz is zero then host has nothing pending
>* and does not need to be signaled.
>*/
> - if (orig_write_sz > pending_sz)
> + if (curr_write_sz - delta > pending_sz)
>   return;
>  
>   /* If pending write will not fit, don't give false hope. */
> - if (hv_get_bytes_to_write(rbi) < pending_sz)
> + if (curr_write_sz <= pending_sz)
>   return;
> +
> + vmbus_setevent(channel);
>   }
>  
> - vmbus_setevent(channel);
>  }

I think this won't work on older versions of Windows where feat_pending_sz
is never set.  On those versions vmbus_setevent needs to always be called.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()

[Bert Kenward]

On 12/02/18 21:03, Boris Brezillon wrote:
> MTD users are no longer checking erase_info->state to determine if the
> erase operation failed or succeeded. Moreover, mtd_erase_callback() is
> now a NOP.
> 
> We can safely get rid of all mtd_erase_callback() calls and all
> erase_info->state assignments. While at it, get rid of the
> erase_info->state field, all MTD_ERASE_XXX definitions and the
> mtd_erase_callback() function.
> 
> Signed-off-by: Boris Brezillon 

For sfc parts:

Acked-by: Bert Kenward 


Thanks,

Bert.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()

[Miquel Raynal]

Hi Boris,

On Tue, 13 Feb 2018 09:17:14 +0100, Boris Brezillon
 wrote:

> On Tue, 13 Feb 2018 08:42:46 +0100
> Miquel Raynal  wrote:
> 
> > Hi Boris,
> > 
> > Just a few comments about the form.
> > 
> > Otherwise:
> > Reviewed-by: Miquel Raynal 
> > 
> >   
> > > diff --git a/drivers/mtd/devices/lart.c b/drivers/mtd/devices/lart.c
> > > index 555b94406e0b..3d6c8ffd351f 100644
> > > --- a/drivers/mtd/devices/lart.c
> > > +++ b/drivers/mtd/devices/lart.c
> > > @@ -415,7 +415,6 @@ static int flash_erase (struct mtd_info *mtd,struct 
> > > erase_info *instr)
> > >{
> > >   if (!erase_block (addr))
> > > {
> > > -  instr->state = MTD_ERASE_FAILED;
> > >return (-EIO);
> > > }
> > 
> > You can also safely remove these '{' '}'  
> 
> Well, this patch is not about fixing coding style issues, otherwise I'd
> have a lot more work on this driver :-)

Sure, I was not referring to the weird style but just that you switch
from two to one line in the block, thus the braces are not needed
anymore.

> 
> >   
> > >  
> > > @@ -425,9 +424,6 @@ static int flash_erase (struct mtd_info *mtd,struct 
> > > erase_info *instr)
> > >   if (addr == mtd->eraseregions[i].offset + 
> > > (mtd->eraseregions[i].erasesize * mtd->eraseregions[i].numblocks)) i++;
> > >}
> > >  
> > > -   instr->state = MTD_ERASE_DONE;
> > > -   mtd_erase_callback(instr);
> > > -
> > > return (0);
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/mtd_dataflash.c 
> > > b/drivers/mtd/devices/mtd_dataflash.c
> > > index 5dc8bd042cc5..aaaeaae01e1d 100644
> > > --- a/drivers/mtd/devices/mtd_dataflash.c
> > > +++ b/drivers/mtd/devices/mtd_dataflash.c
> > > @@ -220,10 +220,6 @@ static int dataflash_erase(struct mtd_info *mtd, 
> > > struct erase_info *instr)
> > >   }
> > >   mutex_unlock(>lock);
> > >  
> > > - /* Inform MTD subsystem that erase is complete */
> > > - instr->state = MTD_ERASE_DONE;
> > > - mtd_erase_callback(instr);
> > > -
> > >   return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> > > index 0bf4aeaf0cb8..efef43c6684b 100644
> > > --- a/drivers/mtd/devices/mtdram.c
> > > +++ b/drivers/mtd/devices/mtdram.c
> > > @@ -60,8 +60,6 @@ static int ram_erase(struct mtd_info *mtd, struct 
> > > erase_info *instr)
> > >   if (check_offs_len(mtd, instr->addr, instr->len))
> > >   return -EINVAL;
> > >   memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
> > > - instr->state = MTD_ERASE_DONE;
> > > - mtd_erase_callback(instr);
> > 
> > Space ?  
> 
> I could add a blank line, but again, I'm just following the coding style
> in place in this file :-).

Ok.

> 
> >   
> > >   return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> > > index 7287696a21f9..a963c88d392d 100644
> > > --- a/drivers/mtd/devices/phram.c
> > > +++ b/drivers/mtd/devices/phram.c
> > > @@ -44,8 +44,6 @@ static int phram_erase(struct mtd_info *mtd, struct 
> > > erase_info *instr)
> > >* I don't feel at all ashamed. This kind of thing is possible anyway
> > >* with flash, but unlikely.
> > >*/
> > 
> > Not sure this comment is still relevant? Maybe you could remove it
> > or at least change it? 
> >   
> > > - instr->state = MTD_ERASE_DONE;
> > > - mtd_erase_callback(instr);
> > 
> > Space ?
> >   
> > >   return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
> > > index cadea0620cd0..5d842cbca3de 100644
> > > --- a/drivers/mtd/devices/pmc551.c
> > > +++ b/drivers/mtd/devices/pmc551.c
> > > @@ -184,12 +184,10 @@ static int pmc551_erase(struct mtd_info *mtd, 
> > > struct erase_info *instr)
> > >   }
> > >  
> > >out:
> > > - instr->state = MTD_ERASE_DONE;
> > >  #ifdef CONFIG_MTD_PMC551_DEBUG
> > >   printk(KERN_DEBUG "pmc551_erase() done\n");
> > >  #endif
> > >  
> > > - mtd_erase_callback(instr);
> > >   return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/powernv_flash.c 
> > > b/drivers/mtd/devices/powernv_flash.c
> > > index 26f9feaa5d17..5f383630c16f 100644
> > > --- a/drivers/mtd/devices/powernv_flash.c
> > > +++ b/drivers/mtd/devices/powernv_flash.c
> > > @@ -175,19 +175,12 @@ static int powernv_flash_erase(struct mtd_info 
> > > *mtd, struct erase_info *erase)
> > >  {
> > >   int rc;
> > >  
> > > - erase->state = MTD_ERASING;
> > > -
> > >   /* todo: register our own notifier to do a true async implementation */
> > >   rc =  powernv_flash_async_op(mtd, FLASH_OP_ERASE, erase->addr,
> > >   erase->len, NULL, NULL);
> > 
> > Are you sure this is still needed? Maybe this should go away in your
> > first patch?  
> 
> Hm, indeed. This comment should be dropped.
> 
> >   
> > > -
> > > - if (rc) {
> > > + if (rc)
> > >   erase->fail_addr = erase->addr;
> > > -  

Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()

[Boris Brezillon]

On Tue, 13 Feb 2018 08:42:46 +0100
Miquel Raynal  wrote:

> Hi Boris,
> 
> Just a few comments about the form.
> 
> Otherwise:
> Reviewed-by: Miquel Raynal 
> 
> 
> > diff --git a/drivers/mtd/devices/lart.c b/drivers/mtd/devices/lart.c
> > index 555b94406e0b..3d6c8ffd351f 100644
> > --- a/drivers/mtd/devices/lart.c
> > +++ b/drivers/mtd/devices/lart.c
> > @@ -415,7 +415,6 @@ static int flash_erase (struct mtd_info *mtd,struct 
> > erase_info *instr)
> >  {
> > if (!erase_block (addr))
> >   {
> > -instr->state = MTD_ERASE_FAILED;
> >  return (-EIO);
> >   }  
> 
> You can also safely remove these '{' '}'

Well, this patch is not about fixing coding style issues, otherwise I'd
have a lot more work on this driver :-)

> 
> >  
> > @@ -425,9 +424,6 @@ static int flash_erase (struct mtd_info *mtd,struct 
> > erase_info *instr)
> > if (addr == mtd->eraseregions[i].offset + 
> > (mtd->eraseregions[i].erasesize * mtd->eraseregions[i].numblocks)) i++;
> >  }
> >  
> > -   instr->state = MTD_ERASE_DONE;
> > -   mtd_erase_callback(instr);
> > -
> > return (0);
> >  }
> >  
> > diff --git a/drivers/mtd/devices/mtd_dataflash.c 
> > b/drivers/mtd/devices/mtd_dataflash.c
> > index 5dc8bd042cc5..aaaeaae01e1d 100644
> > --- a/drivers/mtd/devices/mtd_dataflash.c
> > +++ b/drivers/mtd/devices/mtd_dataflash.c
> > @@ -220,10 +220,6 @@ static int dataflash_erase(struct mtd_info *mtd, 
> > struct erase_info *instr)
> > }
> > mutex_unlock(>lock);
> >  
> > -   /* Inform MTD subsystem that erase is complete */
> > -   instr->state = MTD_ERASE_DONE;
> > -   mtd_erase_callback(instr);
> > -
> > return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> > index 0bf4aeaf0cb8..efef43c6684b 100644
> > --- a/drivers/mtd/devices/mtdram.c
> > +++ b/drivers/mtd/devices/mtdram.c
> > @@ -60,8 +60,6 @@ static int ram_erase(struct mtd_info *mtd, struct 
> > erase_info *instr)
> > if (check_offs_len(mtd, instr->addr, instr->len))
> > return -EINVAL;
> > memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
> > -   instr->state = MTD_ERASE_DONE;
> > -   mtd_erase_callback(instr);  
> 
> Space ?

I could add a blank line, but again, I'm just following the coding style
in place in this file :-).

> 
> > return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> > index 7287696a21f9..a963c88d392d 100644
> > --- a/drivers/mtd/devices/phram.c
> > +++ b/drivers/mtd/devices/phram.c
> > @@ -44,8 +44,6 @@ static int phram_erase(struct mtd_info *mtd, struct 
> > erase_info *instr)
> >  * I don't feel at all ashamed. This kind of thing is possible anyway
> >  * with flash, but unlikely.
> >  */  
> 
> Not sure this comment is still relevant? Maybe you could remove it
> or at least change it? 
> 
> > -   instr->state = MTD_ERASE_DONE;
> > -   mtd_erase_callback(instr);  
> 
> Space ?
> 
> > return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
> > index cadea0620cd0..5d842cbca3de 100644
> > --- a/drivers/mtd/devices/pmc551.c
> > +++ b/drivers/mtd/devices/pmc551.c
> > @@ -184,12 +184,10 @@ static int pmc551_erase(struct mtd_info *mtd, struct 
> > erase_info *instr)
> > }
> >  
> >out:
> > -   instr->state = MTD_ERASE_DONE;
> >  #ifdef CONFIG_MTD_PMC551_DEBUG
> > printk(KERN_DEBUG "pmc551_erase() done\n");
> >  #endif
> >  
> > -   mtd_erase_callback(instr);
> > return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/powernv_flash.c 
> > b/drivers/mtd/devices/powernv_flash.c
> > index 26f9feaa5d17..5f383630c16f 100644
> > --- a/drivers/mtd/devices/powernv_flash.c
> > +++ b/drivers/mtd/devices/powernv_flash.c
> > @@ -175,19 +175,12 @@ static int powernv_flash_erase(struct mtd_info *mtd, 
> > struct erase_info *erase)
> >  {
> > int rc;
> >  
> > -   erase->state = MTD_ERASING;
> > -
> > /* todo: register our own notifier to do a true async implementation */
> > rc =  powernv_flash_async_op(mtd, FLASH_OP_ERASE, erase->addr,
> > erase->len, NULL, NULL);  
> 
> Are you sure this is still needed? Maybe this should go away in your
> first patch?

Hm, indeed. This comment should be dropped.

> 
> > -
> > -   if (rc) {
> > +   if (rc)
> > erase->fail_addr = erase->addr;
> > -   erase->state = MTD_ERASE_FAILED;
> > -   } else {
> > -   erase->state = MTD_ERASE_DONE;
> > -   }
> > -   mtd_erase_callback(erase);
> > +
> > return rc;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
> > index 0ec85f316d24..2f05e1801047 100644
> > --- a/drivers/mtd/devices/slram.c
> > +++ b/drivers/mtd/devices/slram.c
> > @@ -88,8 +88,6 @@ static int slram_erase(struct mtd_info *mtd, struct 
> > erase_info *instr)
> >  * I