RE: [EXT] [PATCH v1 2/3] scsi: ufs: add error handling of auto-hibern8
Hi, Stanley
Thanks for reply.
>
>On Mon, 2019-05-13 at 18:21 +, Bean Huo (beanhuo) wrote:
>> Hi, Stanley
>>
>> >+
>> >+static inline bool ufshcd_is_auto_hibern8_error(struct ufs_hba *hba,
>> >+ u32 intr_mask)
>> >+{
>> >+ return (ufshcd_is_auto_hibern8_supported(hba) &&
>> >+ !hba->uic_async_done &&
>>
>> Here check if uic_async_done is NULL, no big problem so far, but not safe
>enough.
>> How about setting a flag in ufshcd_auto_hibern8_enable(),
>
>>
>> I concern about how to compatible with auto_hibern8 disabled condition.
>
>Currently auto-hibern8 disabling method is not implemented in mainstream,
>so an "enabling" flag may looks redundant unless disabling path is really
>existed.
>
Did you try to update Auto-Hibernate Idle Timer with 0 through '/sys' (scsi:
ufs: Add support for Auto-Hibernate Idle Timer)?
I don't know if this will disable your UFS controller Auto-Hibernate.
If having a look at UFS host Spec, software writes “0” to disable
Auto-Hibernate Idle Timer.
Sorry I cannot verify this on my platform since it doesn't support
auto-hibernate.
>I agree that checking hba->uic_async_done here does not look so intuitive.
>However even if auto-hibern8 is disabled, these checks could be safe enough
>because both "UIC_HIBERNATE_ENTER" and "UIC_HIBERNATE_EXIT" are
>raised only if "manual-hibernate" is performed, and in this case hba-
>>uic_async_done shall be true.
>
Yes, most of cases ,this is no problem.
>Anything else or corner case I missed?
>
The others are fine. I only concern checking hba->uic_async_done.
//Bean
Re: [PATCH v2 0/2] sd: Rely on the driver core for asynchronous probing
Bart, > This patch series makes the sd driver rely on the driver core for > asynchronous probing. Although it's probably too late to submit this > patch series to Linus during the v5.2 merge window, I want to make > these patches available for review now. Applied to 5.3/scsi-queue (by hand). Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCHv2] osst: kill obsolete driver
Hannes, > The osst driver is becoming obsolete, as the manufacturer went out of > business ages ago, and the maintainer has no means of testing any > improvements anymore. Plus these days flash drives are cheaper and > offer a higher capacity. So drop it completely. Applied to 5.3/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 0/7] hpsa updates
Don, > Don Brace (7): > hpsa: correct simple mode > hpsa: use local workqueues instead of system workqueues > hpsa: check for tag collision > hpsa: wait longer for ptraid commands > hpsa: do-not-complete-cmds-for-deleted-devices > hpsa: correct device resets > hpsa: update driver version Applied to 5.3/scsi-queue, thanks. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 0/2] sd: Rely on the driver core for asynchronous probing
Pavel, > Previous versions broke suspend for me on x60... so I guess I should > test -next tommorow or day after that to see if it works now...? I won't push the 5.3 staging tree out until the merge window is over early next week. But it would be much appreciated if you could verify Bart's fix. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 0/2] sd: Rely on the driver core for asynchronous probing
Hi! > > This patch series makes the sd driver rely on the driver core for > > asynchronous probing. Although it's probably too late to submit this > > patch series to Linus during the v5.2 merge window, I want to make > > these patches available for review now. > > Applied to 5.3/scsi-queue (by hand). Thanks! Previous versions broke suspend for me on x60... so I guess I should test -next tommorow or day after that to see if it works now...? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v2 00/21] megaraid_sas: Driver updates
Shivasharan, > Update megaraid_sas to version 07.708.03.00. Applied to 5.3/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
RE: [EXT] [PATCH v1 2/3] scsi: ufs: add error handling of auto-hibern8
Hi Bean,
On Tue, 2019-05-14 at 11:14 +, Bean Huo (beanhuo) wrote:
> Hi, Stanley
> Thanks for reply.
>
> >
> >On Mon, 2019-05-13 at 18:21 +, Bean Huo (beanhuo) wrote:
> >> Hi, Stanley
> >>
> >> >+
> >> >+static inline bool ufshcd_is_auto_hibern8_error(struct ufs_hba *hba,
> >> >+ u32 intr_mask)
> >> >+{
> >> >+ return (ufshcd_is_auto_hibern8_supported(hba) &&
> >> >+ !hba->uic_async_done &&
> >>
> >> Here check if uic_async_done is NULL, no big problem so far, but not safe
> >enough.
> >> How about setting a flag in ufshcd_auto_hibern8_enable(),
> >
> >>
> >> I concern about how to compatible with auto_hibern8 disabled condition.
> >
> >Currently auto-hibern8 disabling method is not implemented in mainstream,
> >so an "enabling" flag may looks redundant unless disabling path is really
> >existed.
> >
> Did you try to update Auto-Hibernate Idle Timer with 0 through '/sys' (scsi:
> ufs: Add support for Auto-Hibernate Idle Timer)?
> I don't know if this will disable your UFS controller Auto-Hibernate.
> If having a look at UFS host Spec, software writes “0” to disable
> Auto-Hibernate Idle Timer.
> Sorry I cannot verify this on my platform since it doesn't support
> auto-hibernate.
>
Sorry I missed this /sys interface for Auto-Hibernate control.
Yes, I have tested "Auto-Hibernate disabled" case, in this case,
UIC_HIBERNATE_ENTER and UIC_HIBERNATE_EXIT interrupts comes only if
Manual-Hibernate is performed and waiting for completion. Both
interrupts will not be identified as Auto-Hibernate errors by checking
hba->uic_async_done.
As for your concerning, I would like to make "Auto-Hibernate error
detection" more precise in next version: Use below conditions instead of
checking hba->uic_async_done:
As-is:
static inline bool ufshcd_is_auto_hibern8_error(struct ufs_hba *hba,
u32 intr_mask)
{
return (ufshcd_is_auto_hibern8_supported(hba) &&
!hba->uic_async_done &&
(intr_mask & UFSHCD_UIC_AH8_ERROR_MASK));
}
To-be:
static bool ufshcd_is_auto_hibern8_error(struct ufs_hba *hba,
u32 intr_mask)
{
if (!ufshcd_is_auto_hibern8_supported(hba))
return false;
if (!(intr_mask & UFSHCD_UIC_AH8_ERROR_MASK))
return false;
if (hba->active_uic_cmd &&
((hba->active_uic_cmd->command == UIC_CMD_DME_HIBER_ENTER) ||
(hba->active_uic_cmd->command == UIC_CMD_DME_HIBER_EXIT)))
return false;
return true;
}
What would you think about this change?
>
> >I agree that checking hba->uic_async_done here does not look so intuitive.
> >However even if auto-hibern8 is disabled, these checks could be safe enough
> >because both "UIC_HIBERNATE_ENTER" and "UIC_HIBERNATE_EXIT" are
> >raised only if "manual-hibernate" is performed, and in this case hba-
> >>uic_async_done shall be true.
> >
> Yes, most of cases ,this is no problem.
>
> >Anything else or corner case I missed?
> >
> The others are fine. I only concern checking hba->uic_async_done.
>
> //Bean
Many thanks,
Stanley
