Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1302897 > unrolled thread

RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver "Hubbe, Allen" <Allen.Hubbe@emc.com> - 2016-01-06 18:00 +0100
    Re: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver Jon Mason <jdmason@kudzu.us> - 2016-01-06 18:50 +0100
      RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver "Yu, Xiangliang" <Xiangliang.Yu@amd.com> - 2016-01-07 04:00 +0100

#1302897 — RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver

RnJvbTogSm9uIE1hc29uIDxqZG1hc29uQGt1ZHp1LnVzPjoNCj4gT24gV2VkLCBEZWMgMjMsIDIw
MTUgYXQgODo0MiBBTSwgWGlhbmdsaWFuZyBZdSA8WGlhbmdsaWFuZy5ZdUBhbWQuY29tPg0KPiB3
cm90ZToNCg0KPiA+ICsjZGVmaW5lIG5kZXZfcGRldihuZGV2KSAoKG5kZXYpLT5udGIucGRldikN
Cj4gPiArI2RlZmluZSBuZGV2X25hbWUobmRldikgcGNpX25hbWUobmRldl9wZGV2KG5kZXYpKQ0K
PiA+ICsjZGVmaW5lIG5kZXZfZGV2KG5kZXYpICgmbmRldl9wZGV2KG5kZXYpLT5kZXYpDQo+ID4g
KyNkZWZpbmUgbnRiX25kZXYobnRiKSBjb250YWluZXJfb2YobnRiLCBzdHJ1Y3QgYW1kX250Yl9k
ZXYsIG50YikNCj4gPiArI2RlZmluZSBoYl9uZGV2KHdvcmspIGNvbnRhaW5lcl9vZih3b3JrLCBz
dHJ1Y3QgYW1kX250Yl9kZXYsDQo+IGhiX3RpbWVyLndvcmspDQo+ID4gKyNkZWZpbmUgbnRiX2hv
dHBsdWdfbmRldihjb250ZXh0KSAoY29udGFpbmVyX29mKChjb250ZXh0KSwgICAgIFwNCj4gPiAr
ICAgICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgbnRiX2FjcGlfaG90cGx1Z19jb250ZXh0LCBo
cCktPm5kZXYpDQo+IA0KPiBTZWVtcyBsaWtlIHRoZXNlIGFyZSBoaWRpbmcgdGhpbmdzIHRvby4g
IFBsZWFzZSB1c2UgdGhlbSBkaXJlY3RseSAob3INCj4gYXQgbGVhc3QgcHV0IHRoZW0gaW4gdGhl
IEMgZmlsZSBhbmQgbm90IHRoZSBoZWFkZXIgZmlsZSkuDQoNCkkgbGlrZSB0aGVzZSBtYWNyb3Mg
Zm9yIHVwL2Rvd24gY2FzdGluZy4gIFB1dHRpbmcgdGhlbSBjbG9zZSB0byB0aGUgc3RydWN0dXJl
IGRlZmluaXRpb24gc2VlbXMgYXBwcm9wcmlhdGUgdG8gbWUsIHRvby4gIEkgd291bGQgcmF0aGVy
IHNlZSB0aGVtIG1vdmVkIHRvIHJpZ2h0IGJlbG93IHRoZSBkZWZpbml0aW9uIG9mIHN0cnVjdCBh
bWRfbnRiX2RldiwgaW5zdGVhZCBvZiB0byB0aGUgYyBmaWxlLiAgVGhhdCBpcyBteSBvcGluaW9u
LCBidXQgSm9uIGNhbiBtYWtlIHRoZSBmaW5hbCBkZWNpc2lvbiBvbiBpdC4NCg0KSG93ZXZlciwg
dGhlc2UgaW4gcGFydGljdWxhciBhcmUgYnVnZ3k6DQoNCj4gPiArI2RlZmluZSBudGJfbmRldihu
dGIpIGNvbnRhaW5lcl9vZihudGIsIHN0cnVjdCBhbWRfbnRiX2RldiwgbnRiKQ0KPiA+ICsjZGVm
aW5lIGhiX25kZXYod29yaykgY29udGFpbmVyX29mKHdvcmssIHN0cnVjdCBhbWRfbnRiX2RldiwN
Cj4gaGJfdGltZXIud29yaykNCg0KTm90ZTogIm50YiIgd2lsbCBiZSByZXBsYWNlZCBpbiBhbGwg
b2NjdXJyZW5jZXMgdG8gdGhlIHJpZ2h0LiAgVGhpcyBvbmx5IHdvcmtzIGlmIHRoZSBuYW1lICJu
dGIiIGlzIHBhc3NlZCBhcyB0aGUgYXJndW1lbnQuICBJZiB0aGUgYXJndW1lbnQgaXMgbmFtZWQg
ImZvbyIsIGl0IHdpbGwgZWl0aGVyIGZhaWwgYXQgY29tcGlsZSB0aW1lIHRvIGZpbmQgdGhlIG1l
bWJlciAiZm9vIiBpbiBzdHJ1Y3QgYW1kX250Yl9kZXYsIG9yIHdvcnNlLCBpdCB3aWxsIGhpZGUg
YSBidWcgYWNjZXNzaW5nIHRoZSB3cm9uZyBtZW1iZXIgb2YgdGhlIHN0cnVjdC4NCg0KUmVuYW1l
IHRoZSBtYWNybyBwYXJhbWV0ZXIgX19udGIuDQoNCkFsbGVuDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1302924

On Wed, Jan 6, 2016 at 11:52 AM, Hubbe, Allen <Allen.Hubbe@emc.com> wrote:
> From: Jon Mason <jdmason@kudzu.us>:
>> On Wed, Dec 23, 2015 at 8:42 AM, Xiangliang Yu <Xiangliang.Yu@amd.com>
>> wrote:
>
>> > +#define ndev_pdev(ndev) ((ndev)->ntb.pdev)
>> > +#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
>> > +#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
>> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
>> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
>> hb_timer.work)
>> > +#define ntb_hotplug_ndev(context) (container_of((context),     \
>> > +                       struct ntb_acpi_hotplug_context, hp)->ndev)
>>
>> Seems like these are hiding things too.  Please use them directly (or
>> at least put them in the C file and not the header file).
>
> I like these macros for up/down casting.  Putting them close to the structure definition seems appropriate to me, too.  I would rather see them moved to right below the definition of struct amd_ntb_dev, instead of to the c file.  That is my opinion, but Jon can make the final decision on it.

My opinion wasn't super strong on these.  If Allen is fine with them,
then good enough for me :)


> However, these in particular are buggy:
>
>> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
>> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
>> hb_timer.work)
>
> Note: "ntb" will be replaced in all occurrences to the right.  This only works if the name "ntb" is passed as the argument.  If the argument is named "foo", it will either fail at compile time to find the member "foo" in struct amd_ntb_dev, or worse, it will hide a bug accessing the wrong member of the struct.
>
> Rename the macro parameter __ntb.

Good call.  Please make the necessary mods Xiangliang.

Thanks,
Jon


> Allen
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/40F65EF2B5E2254199711F58E3ACB84D99EF72E1%40MX104CL02.corp.emc.com.
> For more options, visit https://groups.google.com/d/optout.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303257

PiA+DQo+ID4+ID4gKyNkZWZpbmUgbmRldl9wZGV2KG5kZXYpICgobmRldiktPm50Yi5wZGV2KSAj
ZGVmaW5lDQo+IG5kZXZfbmFtZShuZGV2KQ0KPiA+PiA+ICtwY2lfbmFtZShuZGV2X3BkZXYobmRl
dikpICNkZWZpbmUgbmRldl9kZXYobmRldikNCj4gPj4gPiArKCZuZGV2X3BkZXYobmRldiktPmRl
dikgI2RlZmluZSBudGJfbmRldihudGIpIGNvbnRhaW5lcl9vZihudGIsDQo+ID4+ID4gK3N0cnVj
dCBhbWRfbnRiX2RldiwgbnRiKSAjZGVmaW5lIGhiX25kZXYod29yaykgY29udGFpbmVyX29mKHdv
cmssDQo+ID4+ID4gK3N0cnVjdCBhbWRfbnRiX2RldiwNCj4gPj4gaGJfdGltZXIud29yaykNCj4g
Pj4gPiArI2RlZmluZSBudGJfaG90cGx1Z19uZGV2KGNvbnRleHQpIChjb250YWluZXJfb2YoKGNv
bnRleHQpLCAgICAgXA0KPiA+PiA+ICsgICAgICAgICAgICAgICAgICAgICAgIHN0cnVjdCBudGJf
YWNwaV9ob3RwbHVnX2NvbnRleHQsIGhwKS0+bmRldikNCj4gPj4NCj4gPj4gU2VlbXMgbGlrZSB0
aGVzZSBhcmUgaGlkaW5nIHRoaW5ncyB0b28uICBQbGVhc2UgdXNlIHRoZW0gZGlyZWN0bHkgKG9y
DQo+ID4+IGF0IGxlYXN0IHB1dCB0aGVtIGluIHRoZSBDIGZpbGUgYW5kIG5vdCB0aGUgaGVhZGVy
IGZpbGUpLg0KPiA+DQo+ID4gSSBsaWtlIHRoZXNlIG1hY3JvcyBmb3IgdXAvZG93biBjYXN0aW5n
LiAgUHV0dGluZyB0aGVtIGNsb3NlIHRvIHRoZQ0KPiBzdHJ1Y3R1cmUgZGVmaW5pdGlvbiBzZWVt
cyBhcHByb3ByaWF0ZSB0byBtZSwgdG9vLiAgSSB3b3VsZCByYXRoZXIgc2VlIHRoZW0NCj4gbW92
ZWQgdG8gcmlnaHQgYmVsb3cgdGhlIGRlZmluaXRpb24gb2Ygc3RydWN0IGFtZF9udGJfZGV2LCBp
bnN0ZWFkIG9mIHRvIHRoZQ0KPiBjIGZpbGUuICBUaGF0IGlzIG15IG9waW5pb24sIGJ1dCBKb24g
Y2FuIG1ha2UgdGhlIGZpbmFsIGRlY2lzaW9uIG9uIGl0Lg0KPiANCj4gTXkgb3BpbmlvbiB3YXNu
J3Qgc3VwZXIgc3Ryb25nIG9uIHRoZXNlLiAgSWYgQWxsZW4gaXMgZmluZSB3aXRoIHRoZW0sIHRo
ZW4NCj4gZ29vZCBlbm91Z2ggZm9yIG1lIDopDQoNCkFncmVlIHdpdGggQWxsZW4ncyBvcGluaW9u
Lg0KDQo+IA0KPiA+IEhvd2V2ZXIsIHRoZXNlIGluIHBhcnRpY3VsYXIgYXJlIGJ1Z2d5Og0KPiA+
DQo+ID4+ID4gKyNkZWZpbmUgbnRiX25kZXYobnRiKSBjb250YWluZXJfb2YobnRiLCBzdHJ1Y3Qg
YW1kX250Yl9kZXYsIG50YikNCj4gPj4gPiArI2RlZmluZSBoYl9uZGV2KHdvcmspIGNvbnRhaW5l
cl9vZih3b3JrLCBzdHJ1Y3QgYW1kX250Yl9kZXYsDQo+ID4+IGhiX3RpbWVyLndvcmspDQo+ID4N
Cj4gPiBOb3RlOiAibnRiIiB3aWxsIGJlIHJlcGxhY2VkIGluIGFsbCBvY2N1cnJlbmNlcyB0byB0
aGUgcmlnaHQuICBUaGlzIG9ubHkgd29ya3MNCj4gaWYgdGhlIG5hbWUgIm50YiIgaXMgcGFzc2Vk
IGFzIHRoZSBhcmd1bWVudC4gIElmIHRoZSBhcmd1bWVudCBpcyBuYW1lZCAiZm9vIiwNCj4gaXQg
d2lsbCBlaXRoZXIgZmFpbCBhdCBjb21waWxlIHRpbWUgdG8gZmluZCB0aGUgbWVtYmVyICJmb28i
IGluIHN0cnVjdA0KPiBhbWRfbnRiX2Rldiwgb3Igd29yc2UsIGl0IHdpbGwgaGlkZSBhIGJ1ZyBh
Y2Nlc3NpbmcgdGhlIHdyb25nIG1lbWJlciBvZg0KPiB0aGUgc3RydWN0Lg0KPiA+DQo+ID4gUmVu
YW1lIHRoZSBtYWNybyBwYXJhbWV0ZXIgX19udGIuDQo+IA0KPiBHb29kIGNhbGwuICBQbGVhc2Ug
bWFrZSB0aGUgbmVjZXNzYXJ5IG1vZHMgWGlhbmdsaWFuZy4NCg0KT2sNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web