All of lore.kernel.org
 help / color / mirror / Atom feed
From: Javier Gonzalez <javier@cnexlabs.com>
To: "Matias Bjørling" <mb@lightnvm.io>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [PATCH 03/20] lightnvm: fix capabilities for 2.0 sysfs
Date: Thu, 22 Feb 2018 10:25:47 +0000	[thread overview]
Message-ID: <A949EA48-27DF-4370-B213-3A4CE4E4F7D1@cnexlabs.com> (raw)
In-Reply-To: <616ad3c9-b10c-0439-2d63-5e9e25abce84@lightnvm.io>

DQoNCj4gT24gMjIgRmViIDIwMTgsIGF0IDEwLjM5LCBNYXRpYXMgQmrDuHJsaW5nIDxtYkBsaWdo
dG52bS5pbz4gd3JvdGU6DQo+IA0KPiBPbiAwMi8yMi8yMDE4IDA4OjQ3IEFNLCBKYXZpZXIgR29u
emFsZXogd3JvdGU6DQo+Pj4gT24gMjIgRmViIDIwMTgsIGF0IDA4LjI4LCBNYXRpYXMgQmrDuHJs
aW5nIDxtYkBsaWdodG52bS5pbz4gd3JvdGU6DQo+Pj4gDQo+Pj4+IE9uIDAyLzIxLzIwMTggMTA6
MjYgQU0sIEphdmllciBHb256w6FsZXogd3JvdGU6DQo+Pj4+IEJvdGggMS4yIGFuZCAyLjAgc3Bl
Y3MgZGVmaW5lIGEgZmllbGQgZm9yIG1lZGlhIGFuZCBjb250cm9sbGVyDQo+Pj4+IGNhcGFiaWxp
dGllcy4gQWxzbywgMS4yIGRlZmluZXMgYSBzZXBhcmF0ZSBmaWVsZCBkZWRpY2F0ZWQgdG8gZGV2
aWNlDQo+Pj4+IGNhcGFiaWxpdGllcy4NCj4+Pj4gSW4gMi4wIHN5c2ZzLCB0aGlzIHZhbHVlcyBo
YXZlIGJlZW4gbWl4ZWQuIFJldmVydCB0aGVtIHRvIHRoZSByaWdodA0KPj4+PiB2YWx1ZS4NCj4+
Pj4gU2lnbmVkLW9mZi1ieTogSmF2aWVyIEdvbnrDoWxleiA8amF2aWVyQGNuZXhsYWJzLmNvbT4N
Cj4+Pj4gLS0tDQo+Pj4+ICBkcml2ZXJzL252bWUvaG9zdC9saWdodG52bS5jIHwgMTggKysrKysr
KysrLS0tLS0tLS0tDQo+Pj4+ICAxIGZpbGUgY2hhbmdlZCwgOSBpbnNlcnRpb25zKCspLCA5IGRl
bGV0aW9ucygtKQ0KPj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9udm1lL2hvc3QvbGlnaHRudm0u
YyBiL2RyaXZlcnMvbnZtZS9ob3N0L2xpZ2h0bnZtLmMNCj4+Pj4gaW5kZXggOTY5YmI4NzQ4NTBj
Li41OThhYmJhNjZmNTIgMTAwNjQ0DQo+Pj4+IC0tLSBhL2RyaXZlcnMvbnZtZS9ob3N0L2xpZ2h0
bnZtLmMNCj4+Pj4gKysrIGIvZHJpdmVycy9udm1lL2hvc3QvbGlnaHRudm0uYw0KPj4+PiBAQCAt
OTE0LDggKzkxNCw4IEBAIHN0YXRpYyBzc2l6ZV90IG52bV9kZXZfYXR0cl9zaG93KHN0cnVjdCBk
ZXZpY2UgKmRldiwNCj4+Pj4gICAgICAgIGlmIChzdHJjbXAoYXR0ci0+bmFtZSwgInZlcnNpb24i
KSA9PSAwKSB7DQo+Pj4+ICAgICAgICAgIHJldHVybiBzY25wcmludGYocGFnZSwgUEFHRV9TSVpF
LCAiJXVcbiIsIGRldl9nZW8tPnZlcl9pZCk7DQo+Pj4+IC0gICAgfSBlbHNlIGlmIChzdHJjbXAo
YXR0ci0+bmFtZSwgImNhcGFiaWxpdGllcyIpID09IDApIHsNCj4+Pj4gLSAgICAgICAgcmV0dXJu
IHNjbnByaW50ZihwYWdlLCBQQUdFX1NJWkUsICIldVxuIiwgZGV2X2dlby0+Yy5jYXApOw0KPj4+
PiArICAgIH0gZWxzZSBpZiAoc3RyY21wKGF0dHItPm5hbWUsICJtZWRpYV9jYXBhYmlsaXRpZXMi
KSA9PSAwKSB7DQo+Pj4+ICsgICAgICAgIHJldHVybiBzY25wcmludGYocGFnZSwgUEFHRV9TSVpF
LCAiJXVcbiIsIGRldl9nZW8tPmMubWNjYXApOw0KPj4+PiAgICAgIH0gZWxzZSBpZiAoc3RyY21w
KGF0dHItPm5hbWUsICJyZWFkX3R5cCIpID09IDApIHsNCj4+Pj4gICAgICAgICAgcmV0dXJuIHNj
bnByaW50ZihwYWdlLCBQQUdFX1NJWkUsICIldVxuIiwgZGV2X2dlby0+Yy50cmR0KTsNCj4+Pj4g
ICAgICB9IGVsc2UgaWYgKHN0cmNtcChhdHRyLT5uYW1lLCAicmVhZF9tYXgiKSA9PSAwKSB7DQo+
Pj4+IEBAIC05OTMsOCArOTkzLDggQEAgc3RhdGljIHNzaXplX3QgbnZtX2Rldl9hdHRyX3Nob3df
MTIoc3RydWN0IGRldmljZSAqZGV2LA0KPj4+PiAgICAgICAgICByZXR1cm4gc2NucHJpbnRmKHBh
Z2UsIFBBR0VfU0laRSwgIiV1XG4iLCBkZXZfZ2VvLT5jLnRiZW0pOw0KPj4+PiAgICAgIH0gZWxz
ZSBpZiAoc3RyY21wKGF0dHItPm5hbWUsICJtdWx0aXBsYW5lX21vZGVzIikgPT0gMCkgew0KPj4+
PiAgICAgICAgICByZXR1cm4gc2NucHJpbnRmKHBhZ2UsIFBBR0VfU0laRSwgIjB4JTA4eFxuIiwg
ZGV2X2dlby0+Yy5tcG9zKTsNCj4+Pj4gLSAgICB9IGVsc2UgaWYgKHN0cmNtcChhdHRyLT5uYW1l
LCAibWVkaWFfY2FwYWJpbGl0aWVzIikgPT0gMCkgew0KPj4+PiAtICAgICAgICByZXR1cm4gc2Nu
cHJpbnRmKHBhZ2UsIFBBR0VfU0laRSwgIjB4JTA4eFxuIiwgZGV2X2dlby0+Yy5tY2NhcCk7DQo+
Pj4+ICsgICAgfSBlbHNlIGlmIChzdHJjbXAoYXR0ci0+bmFtZSwgImNhcGFiaWxpdGllcyIpID09
IDApIHsNCj4+Pj4gKyAgICAgICAgcmV0dXJuIHNjbnByaW50ZihwYWdlLCBQQUdFX1NJWkUsICIw
eCUwOHhcbiIsIGRldl9nZW8tPmMuY2FwKTsNCj4+Pj4gICAgICB9IGVsc2UgaWYgKHN0cmNtcChh
dHRyLT5uYW1lLCAibWF4X3BoeXNfc2VjcyIpID09IDApIHsNCj4+Pj4gICAgICAgICAgcmV0dXJu
IHNjbnByaW50ZihwYWdlLCBQQUdFX1NJWkUsICIldVxuIiwgTlZNX01BWF9WTEJBKTsNCj4+Pj4g
ICAgICB9IGVsc2Ugew0KPj4+PiBAQCAtMTA1NSw3ICsxMDU1LDcgQEAgc3RhdGljIHNzaXplX3Qg
bnZtX2Rldl9hdHRyX3Nob3dfMjAoc3RydWN0IGRldmljZSAqZGV2LA0KPj4+PiAgICAvKiBnZW5l
cmFsIGF0dHJpYnV0ZXMgKi8NCj4+Pj4gIHN0YXRpYyBOVk1fREVWX0FUVFJfUk8odmVyc2lvbik7
DQo+Pj4+IC1zdGF0aWMgTlZNX0RFVl9BVFRSX1JPKGNhcGFiaWxpdGllcyk7DQo+Pj4+ICtzdGF0
aWMgTlZNX0RFVl9BVFRSX1JPKG1lZGlhX2NhcGFiaWxpdGllcyk7DQo+Pj4+ICAgIHN0YXRpYyBO
Vk1fREVWX0FUVFJfUk8ocmVhZF90eXApOw0KPj4+PiAgc3RhdGljIE5WTV9ERVZfQVRUUl9STyhy
ZWFkX21heCk7DQo+Pj4+IEBAIC0xMDgwLDEyICsxMDgwLDEyIEBAIHN0YXRpYyBOVk1fREVWX0FU
VFJfMTJfUk8ocHJvZ19tYXgpOw0KPj4+PiAgc3RhdGljIE5WTV9ERVZfQVRUUl8xMl9STyhlcmFz
ZV90eXApOw0KPj4+PiAgc3RhdGljIE5WTV9ERVZfQVRUUl8xMl9STyhlcmFzZV9tYXgpOw0KPj4+
PiAgc3RhdGljIE5WTV9ERVZfQVRUUl8xMl9STyhtdWx0aXBsYW5lX21vZGVzKTsNCj4+Pj4gLXN0
YXRpYyBOVk1fREVWX0FUVFJfMTJfUk8obWVkaWFfY2FwYWJpbGl0aWVzKTsNCj4+Pj4gK3N0YXRp
YyBOVk1fREVWX0FUVFJfMTJfUk8oY2FwYWJpbGl0aWVzKTsNCj4+Pj4gIHN0YXRpYyBOVk1fREVW
X0FUVFJfMTJfUk8obWF4X3BoeXNfc2Vjcyk7DQo+Pj4+ICAgIHN0YXRpYyBzdHJ1Y3QgYXR0cmli
dXRlICpudm1fZGV2X2F0dHJzXzEyW10gPSB7DQo+Pj4+ICAgICAgJmRldl9hdHRyX3ZlcnNpb24u
YXR0ciwNCj4+Pj4gLSAgICAmZGV2X2F0dHJfY2FwYWJpbGl0aWVzLmF0dHIsDQo+Pj4+ICsgICAg
JmRldl9hdHRyX21lZGlhX2NhcGFiaWxpdGllcy5hdHRyLA0KPj4+PiAgICAgICAgJmRldl9hdHRy
X3ZlbmRvcl9vcGNvZGUuYXR0ciwNCj4+Pj4gICAgICAmZGV2X2F0dHJfZGV2aWNlX21vZGUuYXR0
ciwNCj4+Pj4gQEAgLTExMDgsNyArMTEwOCw3IEBAIHN0YXRpYyBzdHJ1Y3QgYXR0cmlidXRlICpu
dm1fZGV2X2F0dHJzXzEyW10gPSB7DQo+Pj4+ICAgICAgJmRldl9hdHRyX2VyYXNlX3R5cC5hdHRy
LA0KPj4+PiAgICAgICZkZXZfYXR0cl9lcmFzZV9tYXguYXR0ciwNCj4+Pj4gICAgICAmZGV2X2F0
dHJfbXVsdGlwbGFuZV9tb2Rlcy5hdHRyLA0KPj4+PiAtICAgICZkZXZfYXR0cl9tZWRpYV9jYXBh
YmlsaXRpZXMuYXR0ciwNCj4+Pj4gKyAgICAmZGV2X2F0dHJfY2FwYWJpbGl0aWVzLmF0dHIsDQo+
Pj4+ICAgICAgJmRldl9hdHRyX21heF9waHlzX3NlY3MuYXR0ciwNCj4+Pj4gICAgICAgIE5VTEws
DQo+Pj4+IEBAIC0xMTM0LDcgKzExMzQsNyBAQCBzdGF0aWMgTlZNX0RFVl9BVFRSXzIwX1JPKHJl
c2V0X21heCk7DQo+Pj4+ICAgIHN0YXRpYyBzdHJ1Y3QgYXR0cmlidXRlICpudm1fZGV2X2F0dHJz
XzIwW10gPSB7DQo+Pj4+ICAgICAgJmRldl9hdHRyX3ZlcnNpb24uYXR0ciwNCj4+Pj4gLSAgICAm
ZGV2X2F0dHJfY2FwYWJpbGl0aWVzLmF0dHIsDQo+Pj4+ICsgICAgJmRldl9hdHRyX21lZGlhX2Nh
cGFiaWxpdGllcy5hdHRyLA0KPj4+PiAgICAgICAgJmRldl9hdHRyX2dyb3Vwcy5hdHRyLA0KPj4+
PiAgICAgICZkZXZfYXR0cl9wdW5pdHMuYXR0ciwNCj4+PiANCj4+PiBXaXRoIHRoZSBtY2NhcCBj
aGFuZ2VzLCBpdCBzaG91bGQgbWFrZSBzZW5zZSB0byBrZWVwIHRoZSBjYXBhYmlsaXRpZXMNCj4+
PiBhcyBpcy4NCj4+IFRoZSBjaGFuZ2UgYWRkcyBtY2NhcCwgYnV0IHN5c2ZzIHBvaW50cyB0byBj
YXAsIHdoaWNoIGlzIHdyb25nLiBUaGlzDQo+PiBwYXRjaCBpcyBuZWVkZWQuIE90aGVyd2lzZSwg
d2UgY2hhbmdlIHRoZSBuYW1lIG9mIG1jY2FwIHRvIGNhcCwgd2hpY2gNCj4+IGlzIF92ZXJ5XyBj
b25mdXNpbmcgdG8gcGVvcGxlIGZhbWlsaWFyIHRvIGJvdGggc3BlY3MuIFdlIGNhbiBjaGFuZ2UN
Cj4+IHRoZSBuYW1lIG9mIG1jY2FwIHRvIGNhcCBpbiBhIGZ1dHVyZSBzcGVjIHJldmlzaW9uLg0K
Pj4gSmF2aWVyDQo+IA0KPiBUaGluayBvZiB0aGUgc3lzZnMgY2FwYWJpbGl0aWVzIGFzIGFuIGFi
c3RyYWN0IHZhbHVlIHRoYXQgZGVmaW5lcyBnZW5lcmljIGNhcGFiaWxpdGllcy4gSXQgaXMgbm90
IGRpcmVjdGx5IHRpZWQgdG8gZWl0aGVyIDEuMiBvciAyLjAuDQoNCknigJltIHRoaW5raW5nIGFi
b3V0IHRoZSB1c2VyIGxvb2tpbmcgYXQgc3lzZnMgYW5kIGF0IHRoZSBzcGVjIGF0IHRoZSBzYW1l
IHRpbWUgLSBJIG15c2VsZiBnZXQgY29uZnVzZWQgd2hlbiBuYW1lcyBkb27igJl0IG1hdGNoLiAN
Cg0KQW55d2F5LCBJ4oCZbGwga2VlcCBpdCB0aGUgd2F5IGl0IHdhcyBhbmQgYWRkIGEgY29tbWVu
dCBmb3IgY2xhcmlmaWNhdGlvbi4gV291bGQgdGhhdCB3b3JrIGZvciB5b3U/DQoNCkphdmllciA=

WARNING: multiple messages have this Message-ID (diff)
From: Javier Gonzalez <javier@cnexlabs.com>
To: "Matias Bjørling" <mb@lightnvm.io>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [PATCH 03/20] lightnvm: fix capabilities for 2.0 sysfs
Date: Thu, 22 Feb 2018 10:25:47 +0000	[thread overview]
Message-ID: <A949EA48-27DF-4370-B213-3A4CE4E4F7D1@cnexlabs.com> (raw)
In-Reply-To: <616ad3c9-b10c-0439-2d63-5e9e25abce84@lightnvm.io>



> On 22 Feb 2018, at 10.39, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> On 02/22/2018 08:47 AM, Javier Gonzalez wrote:
>>> On 22 Feb 2018, at 08.28, Matias Bjørling <mb@lightnvm.io> wrote:
>>> 
>>>> On 02/21/2018 10:26 AM, Javier González wrote:
>>>> Both 1.2 and 2.0 specs define a field for media and controller
>>>> capabilities. Also, 1.2 defines a separate field dedicated to device
>>>> capabilities.
>>>> In 2.0 sysfs, this values have been mixed. Revert them to the right
>>>> value.
>>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>>> ---
>>>>  drivers/nvme/host/lightnvm.c | 18 +++++++++---------
>>>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>>> index 969bb874850c..598abba66f52 100644
>>>> --- a/drivers/nvme/host/lightnvm.c
>>>> +++ b/drivers/nvme/host/lightnvm.c
>>>> @@ -914,8 +914,8 @@ static ssize_t nvm_dev_attr_show(struct device *dev,
>>>>        if (strcmp(attr->name, "version") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->ver_id);
>>>> -    } else if (strcmp(attr->name, "capabilities") == 0) {
>>>> -        return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.cap);
>>>> +    } else if (strcmp(attr->name, "media_capabilities") == 0) {
>>>> +        return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.mccap);
>>>>      } else if (strcmp(attr->name, "read_typ") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.trdt);
>>>>      } else if (strcmp(attr->name, "read_max") == 0) {
>>>> @@ -993,8 +993,8 @@ static ssize_t nvm_dev_attr_show_12(struct device *dev,
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.tbem);
>>>>      } else if (strcmp(attr->name, "multiplane_modes") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mpos);
>>>> -    } else if (strcmp(attr->name, "media_capabilities") == 0) {
>>>> -        return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mccap);
>>>> +    } else if (strcmp(attr->name, "capabilities") == 0) {
>>>> +        return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.cap);
>>>>      } else if (strcmp(attr->name, "max_phys_secs") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", NVM_MAX_VLBA);
>>>>      } else {
>>>> @@ -1055,7 +1055,7 @@ static ssize_t nvm_dev_attr_show_20(struct device *dev,
>>>>    /* general attributes */
>>>>  static NVM_DEV_ATTR_RO(version);
>>>> -static NVM_DEV_ATTR_RO(capabilities);
>>>> +static NVM_DEV_ATTR_RO(media_capabilities);
>>>>    static NVM_DEV_ATTR_RO(read_typ);
>>>>  static NVM_DEV_ATTR_RO(read_max);
>>>> @@ -1080,12 +1080,12 @@ static NVM_DEV_ATTR_12_RO(prog_max);
>>>>  static NVM_DEV_ATTR_12_RO(erase_typ);
>>>>  static NVM_DEV_ATTR_12_RO(erase_max);
>>>>  static NVM_DEV_ATTR_12_RO(multiplane_modes);
>>>> -static NVM_DEV_ATTR_12_RO(media_capabilities);
>>>> +static NVM_DEV_ATTR_12_RO(capabilities);
>>>>  static NVM_DEV_ATTR_12_RO(max_phys_secs);
>>>>    static struct attribute *nvm_dev_attrs_12[] = {
>>>>      &dev_attr_version.attr,
>>>> -    &dev_attr_capabilities.attr,
>>>> +    &dev_attr_media_capabilities.attr,
>>>>        &dev_attr_vendor_opcode.attr,
>>>>      &dev_attr_device_mode.attr,
>>>> @@ -1108,7 +1108,7 @@ static struct attribute *nvm_dev_attrs_12[] = {
>>>>      &dev_attr_erase_typ.attr,
>>>>      &dev_attr_erase_max.attr,
>>>>      &dev_attr_multiplane_modes.attr,
>>>> -    &dev_attr_media_capabilities.attr,
>>>> +    &dev_attr_capabilities.attr,
>>>>      &dev_attr_max_phys_secs.attr,
>>>>        NULL,
>>>> @@ -1134,7 +1134,7 @@ static NVM_DEV_ATTR_20_RO(reset_max);
>>>>    static struct attribute *nvm_dev_attrs_20[] = {
>>>>      &dev_attr_version.attr,
>>>> -    &dev_attr_capabilities.attr,
>>>> +    &dev_attr_media_capabilities.attr,
>>>>        &dev_attr_groups.attr,
>>>>      &dev_attr_punits.attr,
>>> 
>>> With the mccap changes, it should make sense to keep the capabilities
>>> as is.
>> The change adds mccap, but sysfs points to cap, which is wrong. This
>> patch is needed. Otherwise, we change the name of mccap to cap, which
>> is _very_ confusing to people familiar to both specs. We can change
>> the name of mccap to cap in a future spec revision.
>> Javier
> 
> Think of the sysfs capabilities as an abstract value that defines generic capabilities. It is not directly tied to either 1.2 or 2.0.

I’m thinking about the user looking at sysfs and at the spec at the same time - I myself get confused when names don’t match. 

Anyway, I’ll keep it the way it was and add a comment for clarification. Would that work for you?

Javier 

WARNING: multiple messages have this Message-ID (diff)
From: javier@cnexlabs.com (Javier Gonzalez)
Subject: [PATCH 03/20] lightnvm: fix capabilities for 2.0 sysfs
Date: Thu, 22 Feb 2018 10:25:47 +0000	[thread overview]
Message-ID: <A949EA48-27DF-4370-B213-3A4CE4E4F7D1@cnexlabs.com> (raw)
In-Reply-To: <616ad3c9-b10c-0439-2d63-5e9e25abce84@lightnvm.io>



> On 22 Feb 2018,@10.39, Matias Bj?rling <mb@lightnvm.io> wrote:
> 
> On 02/22/2018 08:47 AM, Javier Gonzalez wrote:
>>> On 22 Feb 2018,@08.28, Matias Bj?rling <mb@lightnvm.io> wrote:
>>> 
>>>> On 02/21/2018 10:26 AM, Javier Gonz?lez wrote:
>>>> Both 1.2 and 2.0 specs define a field for media and controller
>>>> capabilities. Also, 1.2 defines a separate field dedicated to device
>>>> capabilities.
>>>> In 2.0 sysfs, this values have been mixed. Revert them to the right
>>>> value.
>>>> Signed-off-by: Javier Gonz?lez <javier at cnexlabs.com>
>>>> ---
>>>>  drivers/nvme/host/lightnvm.c | 18 +++++++++---------
>>>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>>> index 969bb874850c..598abba66f52 100644
>>>> --- a/drivers/nvme/host/lightnvm.c
>>>> +++ b/drivers/nvme/host/lightnvm.c
>>>> @@ -914,8 +914,8 @@ static ssize_t nvm_dev_attr_show(struct device *dev,
>>>>        if (strcmp(attr->name, "version") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->ver_id);
>>>> -    } else if (strcmp(attr->name, "capabilities") == 0) {
>>>> -        return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.cap);
>>>> +    } else if (strcmp(attr->name, "media_capabilities") == 0) {
>>>> +        return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.mccap);
>>>>      } else if (strcmp(attr->name, "read_typ") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.trdt);
>>>>      } else if (strcmp(attr->name, "read_max") == 0) {
>>>> @@ -993,8 +993,8 @@ static ssize_t nvm_dev_attr_show_12(struct device *dev,
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.tbem);
>>>>      } else if (strcmp(attr->name, "multiplane_modes") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mpos);
>>>> -    } else if (strcmp(attr->name, "media_capabilities") == 0) {
>>>> -        return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mccap);
>>>> +    } else if (strcmp(attr->name, "capabilities") == 0) {
>>>> +        return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.cap);
>>>>      } else if (strcmp(attr->name, "max_phys_secs") == 0) {
>>>>          return scnprintf(page, PAGE_SIZE, "%u\n", NVM_MAX_VLBA);
>>>>      } else {
>>>> @@ -1055,7 +1055,7 @@ static ssize_t nvm_dev_attr_show_20(struct device *dev,
>>>>    /* general attributes */
>>>>  static NVM_DEV_ATTR_RO(version);
>>>> -static NVM_DEV_ATTR_RO(capabilities);
>>>> +static NVM_DEV_ATTR_RO(media_capabilities);
>>>>    static NVM_DEV_ATTR_RO(read_typ);
>>>>  static NVM_DEV_ATTR_RO(read_max);
>>>> @@ -1080,12 +1080,12 @@ static NVM_DEV_ATTR_12_RO(prog_max);
>>>>  static NVM_DEV_ATTR_12_RO(erase_typ);
>>>>  static NVM_DEV_ATTR_12_RO(erase_max);
>>>>  static NVM_DEV_ATTR_12_RO(multiplane_modes);
>>>> -static NVM_DEV_ATTR_12_RO(media_capabilities);
>>>> +static NVM_DEV_ATTR_12_RO(capabilities);
>>>>  static NVM_DEV_ATTR_12_RO(max_phys_secs);
>>>>    static struct attribute *nvm_dev_attrs_12[] = {
>>>>      &dev_attr_version.attr,
>>>> -    &dev_attr_capabilities.attr,
>>>> +    &dev_attr_media_capabilities.attr,
>>>>        &dev_attr_vendor_opcode.attr,
>>>>      &dev_attr_device_mode.attr,
>>>> @@ -1108,7 +1108,7 @@ static struct attribute *nvm_dev_attrs_12[] = {
>>>>      &dev_attr_erase_typ.attr,
>>>>      &dev_attr_erase_max.attr,
>>>>      &dev_attr_multiplane_modes.attr,
>>>> -    &dev_attr_media_capabilities.attr,
>>>> +    &dev_attr_capabilities.attr,
>>>>      &dev_attr_max_phys_secs.attr,
>>>>        NULL,
>>>> @@ -1134,7 +1134,7 @@ static NVM_DEV_ATTR_20_RO(reset_max);
>>>>    static struct attribute *nvm_dev_attrs_20[] = {
>>>>      &dev_attr_version.attr,
>>>> -    &dev_attr_capabilities.attr,
>>>> +    &dev_attr_media_capabilities.attr,
>>>>        &dev_attr_groups.attr,
>>>>      &dev_attr_punits.attr,
>>> 
>>> With the mccap changes, it should make sense to keep the capabilities
>>> as is.
>> The change adds mccap, but sysfs points to cap, which is wrong. This
>> patch is needed. Otherwise, we change the name of mccap to cap, which
>> is _very_ confusing to people familiar to both specs. We can change
>> the name of mccap to cap in a future spec revision.
>> Javier
> 
> Think of the sysfs capabilities as an abstract value that defines generic capabilities. It is not directly tied to either 1.2 or 2.0.

I?m thinking about the user looking at sysfs and at the spec at the same time - I myself get confused when names don?t match. 

Anyway, I?ll keep it the way it was and add a comment for clarification. Would that work for you?

Javier 

  reply	other threads:[~2018-02-22 10:25 UTC|newest]

Thread overview: 99+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-21  9:26 [PATCH V2 00/20] lightnvm: pblk: implement 2.0 support Javier González
2018-02-21  9:26 ` Javier González
2018-02-21  9:26 ` [PATCH 01/20] lightnvm: simplify geometry structure Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:25   ` Matias Bjørling
2018-02-22  7:25     ` Matias Bjørling
2018-02-22  7:44     ` Javier Gonzalez
2018-02-22  7:44       ` Javier Gonzalez
2018-02-22 12:22       ` Matias Bjørling
2018-02-22 12:22         ` Matias Bjørling
2018-02-22 14:13         ` Javier Gonzalez
2018-02-22 14:13           ` Javier Gonzalez
2018-02-21  9:26 ` [PATCH 02/20] lightnvm: add controller capabilities to 2.0 Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:26   ` Matias Bjørling
2018-02-22  7:26     ` Matias Bjørling
2018-02-21  9:26 ` [PATCH 03/20] lightnvm: fix capabilities for 2.0 sysfs Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:28   ` Matias Bjørling
2018-02-22  7:28     ` Matias Bjørling
2018-02-22  7:47     ` Javier Gonzalez
2018-02-22  7:47       ` Javier Gonzalez
2018-02-22  9:39       ` Matias Bjørling
2018-02-22  9:39         ` Matias Bjørling
2018-02-22 10:25         ` Javier Gonzalez [this message]
2018-02-22 10:25           ` Javier Gonzalez
2018-02-22 10:25           ` Javier Gonzalez
2018-02-22 11:10           ` Matias Bjørling
2018-02-22 11:10             ` Matias Bjørling
2018-02-22 11:12             ` Javier Gonzalez
2018-02-22 11:12               ` Javier Gonzalez
2018-02-21  9:26 ` [PATCH 04/20] lightnvm: add minor version to generic geometry Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:34   ` Matias Bjørling
2018-02-22  7:34     ` Matias Bjørling
2018-02-22  7:53     ` Javier González
2018-02-22  7:53       ` Javier González
2018-02-21  9:26 ` [PATCH 05/20] lightnvm: rename number of channels and luns Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 06/20] lightnvm: add shorten OCSSD version in geo Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 07/20] lightnvm: rename sect_* to sec_* Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:43   ` Matias Bjørling
2018-02-22  7:43     ` Matias Bjørling
2018-02-21  9:26 ` [PATCH 08/20] lightnvm: complete geo structure with maxoc* Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:45   ` Matias Bjørling
2018-02-22  7:45     ` Matias Bjørling
2018-02-22  7:55     ` Javier Gonzalez
2018-02-22  7:55       ` Javier Gonzalez
2018-02-22  9:45       ` Matias Bjørling
2018-02-22  9:45         ` Matias Bjørling
2018-02-22  9:52         ` Javier Gonzalez
2018-02-22  9:52           ` Javier Gonzalez
2018-02-22 10:00           ` Matias Bjørling
2018-02-22 10:00             ` Matias Bjørling
2018-02-22 10:03             ` Javier Gonzalez
2018-02-22 10:03               ` Javier Gonzalez
2018-02-21  9:26 ` [PATCH 09/20] lightnvm: use generic identify structure Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:47   ` Matias Bjørling
2018-02-22  7:47     ` Matias Bjørling
2018-02-22  7:49     ` Javier González
2018-02-22  7:49       ` Javier González
2018-02-22  9:41       ` Matias Bjørling
2018-02-22  9:41         ` Matias Bjørling
2018-02-21  9:26 ` [PATCH 10/20] lightnvm: pblk: rename ppaf* to addrf* Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:47   ` Matias Bjørling
2018-02-22  7:47     ` Matias Bjørling
2018-02-22  7:47     ` Matias Bjørling
2018-02-21  9:26 ` [PATCH 11/20] lightnvm: pblk: check for supported version Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  7:48   ` Matias Bjørling
2018-02-22  7:48     ` Matias Bjørling
2018-02-21  9:26 ` [PATCH 12/20] lightnvm: complete 2.0 values in sysfs Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 13/20] lightnvm: add support for 2.0 address format Javier González
2018-02-21  9:26   ` Javier González
2018-02-22  9:24   ` Matias Bjørling
2018-02-22  9:24     ` Matias Bjørling
2018-02-21  9:26 ` [PATCH 14/20] lightnvm: make address conversions depend on generic device Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 15/20] nvme: make nvme_get_log_ext available Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 16/20] lightnvm: implement get log report chunk helpers Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 17/20] lightnvm: define chunk states Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 18/20] lightnvm: pblk: implement get log report chunk Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 19/20] lightnvm: pblk: refactor init/exit sequences Javier González
2018-02-21  9:26   ` Javier González
2018-02-21  9:26 ` [PATCH 20/20] lightnvm: pblk: implement 2.0 support Javier González
2018-02-21  9:26   ` Javier González
2018-02-21 14:30   ` Javier González
2018-02-21 14:30     ` Javier González

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=A949EA48-27DF-4370-B213-3A4CE4E4F7D1@cnexlabs.com \
    --to=javier@cnexlabs.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=mb@lightnvm.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.