Re: [PATCH] scsi: associate bio write hint with WRITE CDB

2019-01-10 Thread Alex Lemberg
Hi Randall,

On Fri, 2019-01-04 at 13:22 +0800, Randall Huang wrote:
> On Thu, Jan 03, 2019 at 11:57:38PM -0500, Martin K. Petersen wrote:
> > 
> > Ewan,
> > 
> > > SBC-5 says that support for the grouping function is indicated by the
> > > GROUP_SUP bit in the Extended Inquiry VPD page (86h).  I'm not sure
> > > how many devices actually support that page though.  Probably most
> > > don't.
> > 
> > Several devices support it, albeit for various different purposes. It's
> > one of these wonderful features whose interpretation was left outside
> > the scope of the spec for a long time.
> > 
> > So even though we absolutely and positively need to make setting GROUP
> > NUMBER conditional on GROUP_SUP being reported, we also need additional
> > information from the storage about how the field should be interpreted.
> > 
> > The official way to report hinting is for the device to implement the IO
> > Advice Hints Grouping mode page. I wrote some code to support that but
> > no vendors that I know of ended up actually shipping an implementation.
> > A few implemented my older I/O class proposal but didn't ship that
> > either despite really convincing performance results.
> > 
> > If Randall has access to a device which implements hinting, I'd love to
> > know more.
> I am working on Android phone.
> The idea is to enable write hint for Turbo write UFS feature.
> Turbo write feature in UFS 3.x is under discussion in JEDEC JC-64. 
> This patch is the under-lying framework for supporting this feature.

As far as I know, there are more than one Turbo Write feature
proposals in JEDEC, which are currently under discussion.
For now, not all proposals are using CDB bytes for enabling
TurboWrite.
So, maybe making this change in SCSI is still too early.

> > 
> > -- 
> > Martin K. Petersen  Oracle Linux Engineering



Re: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree

2018-12-04 Thread Alex Lemberg

Hi,


On 11/20/18, 10:07 PM, "linux-mmc-ow...@vger.kernel.org on behalf of Liming 
Sun"  wrote:

Currently the EXT_CSD_CACHE_CTRL register is controlled by API
mmc_card_broken_hpi(), which only works for the quirks settings.
This commit enhances it to use card->ext_csd.hpi instead. This
flag works for both the quirks and the broken-hpi configuration
in the device tree.

Signed-off-by: Liming Sun 
---
 drivers/mmc/core/mmc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index bc1bd2c..2f2b65c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1791,8 +1791,7 @@ static int mmc_init_card(struct mmc_host *host, u32 
ocr,
 * If cache size is higher than 0, this indicates
 * the existence of cache and it can be turned on.
 */
-   if (!mmc_card_broken_hpi(card) &&
-   card->ext_csd.cache_size > 0) {
+   if (card->ext_csd.hpi && card->ext_csd.cache_size > 0) {


Any reason for making a connection between HPI and Cache
in current driver implementation?
BTW, I missed it before, but the previous commit making this connection -
is also doesn't make any sense.
Although by the spec FLUSH_CACHE is interruptible command, I don't think
we should prevent devices with no HPI support, using the Cache.
Please correct me if I wrong, but the current driver implementation allows
to send HPI for BKOPS and Sanitize (on timeout error) commands only -
so why we should limit the FLUSH_CACHE command?


err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_CACHE_CTRL, 1,
card->ext_csd.generic_cmd6_time);
-- 
1.8.3.1





Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

2018-03-08 Thread Alex Lemberg
Hi Linus,

On 3/2/18 4:53 AM, Linus Walleij wrote:
> On Tue, Feb 27, 2018 at 12:33 PM, Harish Jenny K N
>  wrote:
>
>> From: Andrew Gabbasov 
>>
>> Since RPMB area is accessible via special ioctl only and boot areas
>> are unlikely to contain any partitions, exclude them all from listing
>> in /proc/partitions. This will hide them from various user-level
>> software (e.g. fdisk), thus avoiding unnecessary access attempts.
>>
>> Signed-off-by: Andrew Gabbasov 
>> Signed-off-by: Harish Jenny K N 
> Makes sense to me, at least it makes the problem smaller not bigger.
> Reviewed-by: Linus Walleij 
>
>> The main intention of the patch was to not have RPMB device in 
>> /proc/partitions.
>> Boot partitions are also unlikely to have any partitioning, so it made sense 
>> to
>> treat them the same way as RPMB and not list in /proc/partitions.
>> Now I see that RPMB is converted to a character device and this change
>> may not be required for RPMB.
> It certainly does not hurt, even if this code path is not called
> for the RPMB partition anymore (luckily).
>
> On a general note, I do not feel the work with boot partitions or
> the general purpose partitions is finished.
>
> In a lecture I pointed out that:
>
> dd if=/dev/sda of=sda.img
>
> Gives an image of the whole device, and you can restore the
> device by doing the inverse of that command.
>
> For MMC devices,
>
> dd if=/dev/mmcblk0 of=mmcblk0.img
>
> does NOT have the same nice property. Instead, since the
> special partitions are their own devices and not part of the main
> device, they have to be backed up separately.
>
> IMO this breaks the user contract of how a device vs a partition
> works.
>
> What we need to do is make the "special partitions" part of the
> main block device and stop spawning these special block
> devices for each boot partions or general partitions. In addition,
> each of these boot partitions or general partitions will get their
> own block queue and state container which is not cheap in
> runtime memory footprint terms.
>
> So what I want to do (unless someone beats me to it) it to come
> up with some way making boot and general partitions part
> of the main block device. Possibly the core kernel partitioning
> code needs to be augmented. The tentative idea is to just
> put the sectors representing these partitions after the main
> block device and access them from there, with an offset.
I don't think that hiding the Boot and RPMB will resolve the problem
described above.
Boot partition (same as RPMB) in eMMC device is a separate
"physical" partition.
It has its own logical address range and different from general
partition characteristics.
From the protocol - the access to this partition it requires switch
partition command. It can be accesses during the boot sequence
before the general/userdata partition is mounted.
From the device side - it can be managed in totally different manner
(SLC vs. MLC blocks, etc.)
I think it completely makes sense to allow access to Boot partition from the
user space. For example - to allow R/W the boot image.

AFAIK, in case of SCSI/UFS devices - Boot LUN's are represented as
separate block
device partitions (/dev/sdb, dev/sdc...).
Shouldn't we have the same for eMMC?

> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

2018-02-27 Thread Alex Lemberg
Hi Andrew,

While RPMB partition requires special IOCTL, the boot partition is only 
requires "switch partition", which is not unusual operation in eMMC.
Why to prevent users access boot partition?

Thanks,
Alex 

On 2/27/18, 1:34 PM, "linux-mmc-ow...@vger.kernel.org on behalf of Harish 
Jenny K N"  wrote:

From: Andrew Gabbasov 

Since RPMB area is accessible via special ioctl only and boot areas
are unlikely to contain any partitions, exclude them all from listing
in /proc/partitions. This will hide them from various user-level
software (e.g. fdisk), thus avoiding unnecessary access attempts.

Signed-off-by: Andrew Gabbasov 
Signed-off-by: Harish Jenny K N 
---
 drivers/mmc/core/block.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 20135a5..376e47e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2341,7 +2341,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct 
mmc_card *card,
set_disk_ro(md->disk, md->read_only || default_ro);
md->disk->flags = GENHD_FL_EXT_DEVT;
if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
-   md->disk->flags |= GENHD_FL_NO_PART_SCAN;
+   md->disk->flags |= GENHD_FL_NO_PART_SCAN
+  | GENHD_FL_SUPPRESS_PARTITION_INFO;
 
/*
 * As discussed on lkml, GENHD_FL_REMOVABLE should:
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

2018-02-27 Thread Alex Lemberg
While RPMB partition requires special IOCTL

Thanks,
Alex 
On 2/27/18, 1:34 PM, "linux-mmc-ow...@vger.kernel.org on behalf of Harish 
Jenny K N"  wrote:

From: Andrew Gabbasov 

Since RPMB area is accessible via special ioctl only and boot areas
are unlikely to contain any partitions, exclude them all from listing
in /proc/partitions. This will hide them from various user-level
software (e.g. fdisk), thus avoiding unnecessary access attempts.

Signed-off-by: Andrew Gabbasov 
Signed-off-by: Harish Jenny K N 
---
 drivers/mmc/core/block.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 20135a5..376e47e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2341,7 +2341,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct 
mmc_card *card,
set_disk_ro(md->disk, md->read_only || default_ro);
md->disk->flags = GENHD_FL_EXT_DEVT;
if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
-   md->disk->flags |= GENHD_FL_NO_PART_SCAN;
+   md->disk->flags |= GENHD_FL_NO_PART_SCAN
+  | GENHD_FL_SUPPRESS_PARTITION_INFO;
 
/*
 * As discussed on lkml, GENHD_FL_REMOVABLE should:
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




Re: [PATCH] mmc: core: add auto bkops support

2016-06-27 Thread Alex Lemberg
Hi Ulf and Adrian,


On 6/27/16, 12:08 PM, "Ulf Hansson"  wrote:

>[...]
>
 If I am not wrong, in current implementation of runtime suspend,
 the driver stops BKOPS (send HPI) just before sending sleep command,
 see _mmc_suspend(), depends on “MMC_CAP_AGGRESSIVE_PM” flag.
 In this case, the eMMC device will not have enough time to perform internal
 BKOPS in both – Manual and Auto BKOPS configurations.

>>>
>>> ye, so it seems a pre-exiting issue before introducing auto bkops?
>>> I think we can push another patch to improve it but not handling
>>> it for this $SUBJECT, does it sound ok to you?
>>
>> Runtime suspend for eMMC has a default auto-suspend delay of 3 seconds
>> (refer mmc_blk_probe()).  Isn't that when auto bkops would happen?
>
>That's correct.
>
>So perhaps we should extend the default auto-suspend delay?

Due to the differences in flash management and technologies,
I suggest to make this value configurable, and not hard coded.
I believe it should be configured per vendor BKOPS needs.
The default value still can be set to 3 seconds, as done today…

>
>The SD case actually have the same issue, as I some background
>operations exists there as well.
>
>[...]
>
>Kind regards
>Uffe



Re: [PATCH] mmc: core: add auto bkops support

2016-06-22 Thread Alex Lemberg


On 6/22/16, 5:28 PM, "Ulf Hansson"  wrote:

>On 22 June 2016 at 16:20, Alex Lemberg  wrote:
>>
>>
>> On 6/22/16, 1:21 PM, "Ulf Hansson"  wrote:
>>
>>>On 13 June 2016 at 14:25, Adrian Hunter  wrote:
>>>> On 13/06/16 11:58, Shawn Lin wrote:
>>>>> 在 2016/6/13 16:17, Adrian Hunter 写道:
>>>>>> On 13/06/16 10:48, Shawn Lin wrote:
>>>>>>> On 2016/6/13 14:29, Adrian Hunter wrote:
>>>>>>>> On 06/06/16 06:07, Shawn Lin wrote:
>>>>>>>>> JEDEC eMMC v5.1 introduce an autonomously initiated method
>>>>>>>>> for background operations.
>>>>>>>>>
>>>>>>>>> Host that wants to enable the device to perform background
>>>>>>>>> operations during device idle time, should signal the device
>>>>>>>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When
>>>>>>>>> this bit is set, the device may start or stop background operations
>>>>>>>>> whenever it sees fit, without any notification to the host.
>>>>>>>>>
>>>>>>>>> When AUTO_EN bit is set, the host should keep the device power
>>>>>>>>> active. The host may set or clear this bit at any time based on
>>>>>>>>> its power constraints or other considerations.
>>>>>>>>>
>>>>>>>>> Currently the manual bkops is only be used under the async req
>>>>>>>>> circumstances and it's a bit complicated to be controlled as the
>>>>>>>>> perfect method is that we should do some idle monitor just as rpm
>>>>>>>>> and send HPI each time if receiving rd/wr req. But it will impact
>>>>>>>>> performance significantly, especially for random iops since the
>>>>>>>>> weight of executing HPI against r/w small piece of LBAs is
>>>>>>>>> nonnegligible.
>>>>>>>>>
>>>>>>>>> So we now prefer to select the auto one unconditionally if supported
>>>>>>>>> which makes it as simple as possible. It should really good enough
>>>>>>>>> for devices to manage its internal policy for bkops rather than the
>>>>>>>>> host, which makes us believe that we could achieve the best
>>>>>>>>> performance for all the devices implementing auto bkops and the only
>>>>>>>>> thing we should do is to disable it when cutting off the power.
>>>>>>>>
>>>>>>>> Do you know if there is really a requirement to do that?
>>>>>>>
>>>>>>> Even without bkops enable, no matter for manual or auto one, FTL should
>>>>>>> always do bkops like GC internally when needed to guarantee the
>>>>>>> performance and balance the wear leveling. What I thought to do is to
>>>>>>> make it more explicitly.
>>>>>>>
>>>>>>> Because then, what
>>>>>>>> is the point of power off notification?
>>>>>>>
>>>>>>> When power off notification is sent, bkops will be stopped
>>>>>>> in _mmc_suspend. So I don't undertand your point here?
>>>>>>
>>>>>> I am trying to understand why we need to do anything for auto bkops.
>>>>>> Since AUTO_EN is persistent, we can leave the decision whether to turn 
>>>>>> it on
>>>>>> to whomever provisions the device. Then we just leave it alone.
>>>>>>
>>>>>
>>>>> Hrm..
>>>>>
>>>>> one possible way is to control it by mmc-utils on
>>>>> user space?  So we should add a cmd for mmc-utils
>>>>> there?
>>>>
>>>> That would be consistent with manual bkops.
>>>>
>>>
>> >From my first impression I agree, as that is the policy we have been
>>>sticking to when writing to persistent EXT_CSD registers.
>>>Although, in this case, I am actually wondering on what is the best approach.
>>>
>>>Is there really ever a case when we don't want auto BKOPS to be default 
>>>enabled?
>>>I think BKOPS is a fundamental feature of an FTL and I can't see a
>>>reason to why we need to involve mmc-utils/userspace to enable it. Am
>>>I wrong?
>>
>> The even worst case is – involve mmc-utils/userspace to DISABLE it.
>> I think this register need to be set by vendor and no need to be changed on 
>> runtime.
>
>If it is set by the Vendor, that's of course the best.

It can be set by Storage Vendor.
According to the spec, the default value of this bit is vendor specific.

>
>Are you saying that we shouldn't enable it during the card init
>sequence from the kernel, in case it is disabled?

No.
By the spec – a Host that wants to enable the device to perform
background operations during device idle time, should signal the 
device by setting AUTO_EN in BKOPS_EN field [EXT_CSD byte 163] to 1b.

>
>Kind regards
>Uffe



Re: [PATCH] mmc: core: add auto bkops support

2016-06-22 Thread Alex Lemberg


On 6/22/16, 1:21 PM, "Ulf Hansson"  wrote:

>On 13 June 2016 at 14:25, Adrian Hunter  wrote:
>> On 13/06/16 11:58, Shawn Lin wrote:
>>> 在 2016/6/13 16:17, Adrian Hunter 写道:
 On 13/06/16 10:48, Shawn Lin wrote:
> On 2016/6/13 14:29, Adrian Hunter wrote:
>> On 06/06/16 06:07, Shawn Lin wrote:
>>> JEDEC eMMC v5.1 introduce an autonomously initiated method
>>> for background operations.
>>>
>>> Host that wants to enable the device to perform background
>>> operations during device idle time, should signal the device
>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When
>>> this bit is set, the device may start or stop background operations
>>> whenever it sees fit, without any notification to the host.
>>>
>>> When AUTO_EN bit is set, the host should keep the device power
>>> active. The host may set or clear this bit at any time based on
>>> its power constraints or other considerations.
>>>
>>> Currently the manual bkops is only be used under the async req
>>> circumstances and it's a bit complicated to be controlled as the
>>> perfect method is that we should do some idle monitor just as rpm
>>> and send HPI each time if receiving rd/wr req. But it will impact
>>> performance significantly, especially for random iops since the
>>> weight of executing HPI against r/w small piece of LBAs is
>>> nonnegligible.
>>>
>>> So we now prefer to select the auto one unconditionally if supported
>>> which makes it as simple as possible. It should really good enough
>>> for devices to manage its internal policy for bkops rather than the
>>> host, which makes us believe that we could achieve the best
>>> performance for all the devices implementing auto bkops and the only
>>> thing we should do is to disable it when cutting off the power.
>>
>> Do you know if there is really a requirement to do that?
>
> Even without bkops enable, no matter for manual or auto one, FTL should
> always do bkops like GC internally when needed to guarantee the
> performance and balance the wear leveling. What I thought to do is to
> make it more explicitly.
>
> Because then, what
>> is the point of power off notification?
>
> When power off notification is sent, bkops will be stopped
> in _mmc_suspend. So I don't undertand your point here?

 I am trying to understand why we need to do anything for auto bkops.
 Since AUTO_EN is persistent, we can leave the decision whether to turn it 
 on
 to whomever provisions the device. Then we just leave it alone.

>>>
>>> Hrm..
>>>
>>> one possible way is to control it by mmc-utils on
>>> user space?  So we should add a cmd for mmc-utils
>>> there?
>>
>> That would be consistent with manual bkops.
>>
>
>From my first impression I agree, as that is the policy we have been
>sticking to when writing to persistent EXT_CSD registers.
>Although, in this case, I am actually wondering on what is the best approach.
>
>Is there really ever a case when we don't want auto BKOPS to be default 
>enabled?
>I think BKOPS is a fundamental feature of an FTL and I can't see a
>reason to why we need to involve mmc-utils/userspace to enable it. Am
>I wrong?

The even worst case is – involve mmc-utils/userspace to DISABLE it.
I think this register need to be set by vendor and no need to be changed on 
runtime.

>
>Kind regards
>Uffe



Re: [PATCH] mmc: core: add auto bkops support

2016-06-22 Thread Alex Lemberg
HI Shawn,

On 6/21/16, 4:44 AM, "Shawn Lin"  wrote:

>On 2016/6/20 21:33, Alex Lemberg wrote:
>> Hi Shawn,
>>
>> […]
>>
>>>>> +
>>>>> +static int mmc_stop_auto_bkops(struct mmc_card *card)
>>>>> +{
>>>>> + int err = 0;
>>>>> +
>>>>> + if (!card->ext_csd.auto_bkops_en)
>>>>> + return 0;
>>>>> +
>>>>
>>>> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS 
>>>> activity of the device?
>>>>
>>>
>>> Hrmm.. I read the whole section of spec for it, and I did find this
>>> requirement for manul bkops but not for the auto one. So what should we
>>> do if using the auto one?
>>>
>>
>> In case of AUTO BKOPS, the eMMC Device should perform internal GC
>> in the same way as in case of MANUAL BKOPS.
>> The only difference is a host awareness.
>
>agree.
>
>> Although there is no requirement in the spec, I think the driver can
>> give some time to the device to perform/complete its internal GC during the 
>> idle time.
>> Thus I think we can check the BKOPS_STATUS on Runtime suspend.
>
>We shouldn't diable bkops on *runtime* suspend as it's just the right
>time for firmware to do GC. We could consider to check and wait for
>the status when doing poweroff, although it seems firmware should be
>able to accept the disable cmd and deal the on-going work perfectly
>when doing bkops without host's awareness, just the same way as suddent
>power loss cases.

If I am not wrong, in current implementation of runtime suspend, 
the driver stops BKOPS (send HPI) just before sending sleep command,
see _mmc_suspend(), depends on “MMC_CAP_AGGRESSIVE_PM” flag.
In this case, the eMMC device will not have enough time to perform internal 
BKOPS in both – Manual and Auto BKOPS configurations.

For the poweroff, it should be OK with a current implementation of 
PON (mmc_poweroff_notify())

>
>Also I don't know whether the firmware will reflect its status on
>BKOPS_STATUS or not when enabling the auto one. I will do more test.
>
>Anyway, thanks for sharing your thought.
>Also Adrian point out that currently we trigger manual bkosp from
>userspace via mmc-utils, and I agreed we shouldn't force kernel stack
>to enable it defaultly. So I'm prone not to update this $SUBJECT and
>migrate it to mmc-utils later.
>
>>
>> […]
>>
>> Thanks,
>> Alex
>>
>
>
>-- 
>Best Regards
>Shawn Lin
>



Re: [PATCH] mmc: core: add auto bkops support

2016-06-20 Thread Alex Lemberg
Hi Shawn,

[…]

>>> +
>>> +static int mmc_stop_auto_bkops(struct mmc_card *card)
>>> +{
>>> +   int err = 0;
>>> +
>>> +   if (!card->ext_csd.auto_bkops_en)
>>> +   return 0;
>>> +
>>
>> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity 
>> of the device?
>>
>
>Hrmm.. I read the whole section of spec for it, and I did find this
>requirement for manul bkops but not for the auto one. So what should we
>do if using the auto one?
>

In case of AUTO BKOPS, the eMMC Device should perform internal GC 
in the same way as in case of MANUAL BKOPS.
The only difference is a host awareness. 
Although there is no requirement in the spec, I think the driver can
give some time to the device to perform/complete its internal GC during the 
idle time.
Thus I think we can check the BKOPS_STATUS on Runtime suspend.

[…]

Thanks,
Alex



Re: [PATCH] mmc: core: add auto bkops support

2016-06-08 Thread Alex Lemberg
Hi Shawn,

Is the intention in this patch to stop using MANUAL_BKOPS for all eMMC5.1+ 
devices, and only use AUTO_BKOPS?
Please see several questions inline.


On 6/6/16, 6:07 AM, "linux-mmc-ow...@vger.kernel.org on behalf of Shawn Lin" 
 wrote:

>JEDEC eMMC v5.1 introduce an autonomously initiated method
>for background operations.
>
>Host that wants to enable the device to perform background
>operations during device idle time, should signal the device
>by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When
>this bit is set, the device may start or stop background operations
>whenever it sees fit, without any notification to the host.
>
>When AUTO_EN bit is set, the host should keep the device power
>active. The host may set or clear this bit at any time based on
>its power constraints or other considerations.
>
>Currently the manual bkops is only be used under the async req
>circumstances and it's a bit complicated to be controlled as the
>perfect method is that we should do some idle monitor just as rpm
>and send HPI each time if receiving rd/wr req. But it will impact
>performance significantly, especially for random iops since the
>weight of executing HPI against r/w small piece of LBAs is
>nonnegligible.
>
>So we now prefer to select the auto one unconditionally if supported
>which makes it as simple as possible. It should really good enough
>for devices to manage its internal policy for bkops rather than the
>host, which makes us believe that we could achieve the best
>performance for all the devices implementing auto bkops and the only
>thing we should do is to disable it when cutting off the power.
>
>Signed-off-by: Shawn Lin 
>---
>
> drivers/mmc/core/core.c  | 127 +--
> drivers/mmc/core/mmc.c   |  30 ++-
> include/linux/mmc/card.h |   1 +
> include/linux/mmc/core.h |   4 +-
> include/linux/mmc/mmc.h  |   1 +
> 5 files changed, 133 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>index e864187..c2e5a66 100644
>--- a/drivers/mmc/core/core.c
>+++ b/drivers/mmc/core/core.c
>@@ -297,24 +297,13 @@ static int mmc_start_request(struct mmc_host *host, 
>struct mmc_request *mrq)
>   return 0;
> }
> 
>-/**
>- *mmc_start_bkops - start BKOPS for supported cards
>- *@card: MMC card to start BKOPS
>- *@form_exception: A flag to indicate if this function was
>- * called due to an exception raised by the card
>- *
>- *Start background operations whenever requested.
>- *When the urgent BKOPS bit is set in a R1 command response
>- *then background operations should be started immediately.
>-*/
>-void mmc_start_bkops(struct mmc_card *card, bool from_exception)
>+static void  mmc_start_man_bkops(struct mmc_card *card,
>+   bool from_exception)
> {
>   int err;
>   int timeout;
>   bool use_busy_signal;
> 
>-  BUG_ON(!card);
>-
>   if (!card->ext_csd.man_bkops_en || mmc_card_doing_bkops(card))
>   return;
> 
>@@ -347,7 +336,7 @@ void mmc_start_bkops(struct mmc_card *card, bool 
>from_exception)
>   EXT_CSD_BKOPS_START, 1, timeout,
>   use_busy_signal, true, false);
>   if (err) {
>-  pr_warn("%s: Error %d starting bkops\n",
>+  pr_warn("%s: Error %d starting manual bkops\n",
>   mmc_hostname(card->host), err);
>   mmc_retune_release(card->host);
>   goto out;
>@@ -365,6 +354,55 @@ void mmc_start_bkops(struct mmc_card *card, bool 
>from_exception)
> out:
>   mmc_release_host(card->host);
> }
>+
>+static void  mmc_start_auto_bkops(struct mmc_card *card)
>+{
>+  int err;
>+
>+  /* If it's already enable auto_bkops, nothing to do */
>+  if (card->ext_csd.auto_bkops_en)
>+  return;
>+
>+  mmc_claim_host(card->host);
>+
>+  mmc_retune_hold(card->host);
>+
>+  err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>+  EXT_CSD_BKOPS_EN, 2,
>+  card->ext_csd.generic_cmd6_time);
>+  if (err)
>+  pr_warn("%s: Error %d starting auto bkops\n",
>+  mmc_hostname(card->host), err);
>+
>+  card->ext_csd.auto_bkops_en = true;
>+  mmc_card_set_doing_bkops(card);
>+  mmc_retune_release(card->host);
>+  mmc_release_host(card->host);
>+}
>+
>+
>+/**
>+ *mmc_start_bkops - start BKOPS for supported cards
>+ *@card: MMC card to start BKOPS
>+ *@form_exception: A flag to indicate if this function was
>+ * called due to an exception raised by the card
>+ *  @is_auto: A flag to indicate if we should use auto bkops
>+ *
>+ *Start background operations whenever requested.
>+ *When the urgent BKOPS bit is set in a R1 command response
>+ *then background operations should be started immediately, which is
>+ *only needed for man_bkops.
>+*/
>+void mm

RE: [RFC 0/6] mmc: Field Firmware Update

2015-12-28 Thread Alex Lemberg
Hi Ulf,

We succeeded to run FFU via new mmc multi-command ioctl without any code 
modification, 
but only by using Single Sector commands (CMD24).

>From running the FFU and from code review, we see two minor issues in this way 
>of running FFU:
1. There is no support for Multiple Block write commands (CMD25) in existing 
IOCTL implementation - 
seems like there is no polling for the card status on data transfer completion.
(The kernel FFU implementation supports FFU using Multiple Block Write 
commands).
2. As you probably remember, there are two ways to install the new FW in the 
end of FFU process - 
In case MODE_OPERATION_CODES field is not supported by the device, the host 
sets to NORMAL state 
and initiates a CMD0/HW_Reset/Power cycle to install the new firmware.
This sequence cannot be done via multi-command ioctl, and requires manual reset 
of the device/platform.
(The kernel FFU implementation supports both FW install methods).

For running FFU via new mmc multi-command ioctl, we have modified mmc-utils and 
add new functionality for FFU.
Please let us know if you want us to submit the patch for mmc-utils FFU 
functionality via multi-command ioctl.

Best regards,
Alex

> -Original Message-
> From: Holger Schurig [mailto:holgerschu...@gmail.com]
> Sent: Tuesday, December 22, 2015 10:15 AM
> To: Ulf Hansson
> Cc: Avi Shchislowski; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Alex Lemberg; Chris Ball; Baolin Wang
> Subject: Re: [RFC 0/6] mmc: Field Firmware Update
> 
> 
> > Sorry for the delay.
> 
> No problem, I was busy with many other projects as well.
> 
> > My advise right now is to try this out via the mmc ioctl in userspace,
> > yes. Although, if you encounter any issues with that approach that it
> > might not be reliable, I am open to look into the in-kernel solution
> > again.
> 
> I managed to update my Kingston eMMCs with a slighly modified patch to
> what I submitted. I however didn't bother to submit this, as I saw no chance
> of getting it applied.
> 
> Also I once asked in the mailing list if there is some user-space example of
> how to use the multi-block feature that is supposed to enable this, but I
> haven't gotten an answer.
> 
> > Regarding mmc-utils as where I recommend you to implement this, I have
> > been thinking of moving this tool into the tools directory in the
> > kernel.
> 
> Sounds good to me.
> 
> 
> 
> Remotely related:
> 
> Do you know that some google people made their own version of mmc-utils
> for ChromeOS? And the don't seem to give much effort in unification of
> them?  As if the world wouldn't know how hard it can be to re-unite things
> again, Google should know from their custom kernels they use on Android ...
> Sigh.
> 
> 
> So you can
> 
> * git clone git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc-utils.git
> * git clone
> https://chromium.googlesource.com/chromiumos/third_party/mmc-utils
> 
> currently.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/