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

[Yu, Xiangliang]

> >
> >> > +#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 :)

Agree with Allen's opinion.

> 
> > 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.

Ok