On Wed, Feb 19, 2020 at 11:33:21AM +0200, Leon Romanovsky wrote:
> On Wed, Feb 19, 2020 at 10:43:59AM +0200, Kamal Heib wrote:
> > On Tue, Feb 18, 2020 at 06:58:47PM +0200, Leon Romanovsky wrote:
> > > On Tue, Feb 18, 2020 at 11:59:11AM +0200, Kamal Heib wrote:
> > > > Make sure to set the active_{speed, width} attributes to avoid reporting
> > > > the same values regardless of the underlying device.
> > > >
> > > > Fixes: 303ae1cdfdf7 ("rdma/siw: application interface")
> > > > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > > > ---
> > > > V2: Change rc to rv.
> > > > ---
> > > > drivers/infiniband/sw/siw/siw_verbs.c | 7 ++++---
> > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
> > > > index 73485d0da907..d5390d498c61 100644
> > > > --- a/drivers/infiniband/sw/siw/siw_verbs.c
> > > > +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> > > > @@ -165,11 +165,12 @@ int siw_query_port(struct ib_device *base_dev, u8 port,
> > > > struct ib_port_attr *attr)
> > > > {
> > > > struct siw_device *sdev = to_siw_dev(base_dev);
> > > > + int rv;
> > > >
> > > > memset(attr, 0, sizeof(*attr));
> > >
> > > This line should go too. IB/core clears attr prior to call driver.
> > >
> > > Thanks
> > >
> >
> > This can be done in a separate patch as this patch fixes a specific issue.
>
> Whatever works for you, if you don't value your own time, go for it,
> do separate patch for every line you are changing. It just looks crazy
> to see changes like this:
> * changed line
> * line to be changed, but not changed
> * another changed line
>
> Thanks
>
Leon, With all my respect, This isn't your decision what I do and when.
W.R.T your suggestion for removing the memset() line, there are multiple
places in the siw drivers that do memset in the ib_device_ops, with that
said, I think that this need to be done in a separate patch. BTW you can
do the change too ;)
Thanks,
Kamal
> >
> > Thanks,
> > Kamal
> >
> > > >
> > > > - attr->active_speed = 2;
> > > > - attr->active_width = 2;
> > > > + rv = ib_get_eth_speed(base_dev, port, &attr->active_speed,
> > > > + &attr->active_width);
> > > > attr->gid_tbl_len = 1;
> > > > attr->max_msg_sz = -1;
> > > > attr->max_mtu = ib_mtu_int_to_enum(sdev->netdev->mtu);
> > > > @@ -192,7 +193,7 @@ int siw_query_port(struct ib_device *base_dev, u8 port,
> > > > * attr->subnet_timeout = 0;
> > > > * attr->init_type_repy = 0;
> > > > */
> > > > - return 0;
> > > > + return rv;
> > > > }
> > > >
> > > > int siw_get_port_immutable(struct ib_device *base_dev, u8 port,
> > > > --
> > > > 2.21.1
> > > >
