> -----Original Message-----
> From: Johannes Thumshirn [mailto:[email protected]]
> Sent: Monday, January 30, 2017 1:50 AM
> To: Raghava Aditya Renukunta
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; Dave Carroll <[email protected]>; Gana
> Sridaran <[email protected]>; Scott Benesh
> <[email protected]>
> Subject: Re: [PATCH V3 08/24] aacraid: Added support for response path
>
> EXTERNAL EMAIL
>
>
> On Fri, Jan 27, 2017 at 11:28:37AM -0800, Raghava Aditya Renukunta wrote:
> > This patch enables the driver to actually process the I/O, or srb replies
> > from adapter. In addition to any HBA1000 or SmartIOC2000 adapter events.
> >
> > Signed-off-by: Raghava Aditya Renukunta
> <[email protected]>
> > Signed-off-by: Dave Carroll <[email protected]>
> >
> > ---
>
> Sorry for not completely reviewing the patchset before you reposted.
>
> > @@ -475,16 +475,26 @@ int aac_get_containers(struct aac_dev *dev)
> >
> > if (maximum_num_containers < MAXIMUM_NUM_CONTAINERS)
> > maximum_num_containers = MAXIMUM_NUM_CONTAINERS;
> > - fsa_dev_ptr = kzalloc(sizeof(*fsa_dev_ptr) *
> maximum_num_containers,
> > - GFP_KERNEL);
> > - if (!fsa_dev_ptr)
> > - return -ENOMEM;
> > + if ((dev->fsa_dev == NULL) ||
> > + (dev->maximum_num_containers != maximum_num_containers))
> {
> > +
> > + fsa_dev_ptr = dev->fsa_dev;
>
> Comparison has precedence over logical OR. See
> http://en.cppreference.com/w/c/language/operator_precedence
Acknowledged. , I will remove the parentheses.
The idea was to make it easier to read the code , if there were enclosed in
Parenthesis (I dabbled a bit in lisp , and for me brackets makes it easier to
read).
>
> >
> > - dev->fsa_dev = fsa_dev_ptr;
> > - dev->maximum_num_containers = maximum_num_containers;
> > + dev->fsa_dev = kzalloc(sizeof(*fsa_dev_ptr) *
> > + maximum_num_containers, GFP_KERNEL);
>
> kcalloc()?
Yes kcalloc makes sense, but then an additional mem set would be required.
Unless there is a kzcalloc or kczalloc?
> [...]
>
>
> > @@ -251,8 +250,9 @@ static void aac_aif_callback(void *context, struct fib
> * fibptr)
> > BUG_ON(fibptr == NULL);
> > dev = fibptr->dev;
> >
> > - if (fibptr->hw_fib_va->header.XferState &
> > - cpu_to_le32(NoMoreAifDataAvailable)) {
> > + if ((fibptr->hw_fib_va->header.XferState &
> > + cpu_to_le32(NoMoreAifDataAvailable)) ||
> > + dev->sa_firmware) {
>
> Again unnecessary parenthesis.
> > aac_fib_complete(fibptr);
> > aac_fib_free(fibptr);
> > return;
> > @@ -282,8 +282,8 @@ static void aac_aif_callback(void *context, struct fib
> * fibptr)
> > * know there is a response on our normal priority queue. We will pull
> > off
> > * all QE there are and wake up all the waiters before exiting.
> > */
> > -unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
> > - int isAif, int isFastResponse, struct hw_fib *aif_fib)
> > +unsigned int aac_intr_normal(struct aac_dev *dev, u32 index, int isAif,
> > + int isFastResponse, struct hw_fib *aif_fib)
>
> Isn't this exactly the same but now with broken indent?
It is, I tried removing stuff like this before sending the patches, must have
snuck in. I will remove it.
Regards,
Raghava Aditya
> [...]
>
> --
> Johannes Thumshirn Storage
> [email protected] +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
