> -----Original Message----- > From: Sasha Levin [mailto:levinsasha928@xxxxxxxxx] > Sent: Saturday, July 16, 2011 9:02 AM > To: KY Srinivasan > Cc: Christoph Hellwig; gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; Haiyang Zhang > Subject: RE: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices > using the storvsc driver > > On Sat, 2011-07-16 at 12:57 +0000, KY Srinivasan wrote: > > > > > -----Original Message----- > > > From: Christoph Hellwig [mailto:hch@xxxxxxxxxxxxx] > > > Sent: Friday, July 15, 2011 10:05 PM > > > To: KY Srinivasan > > > Cc: gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > > devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; Haiyang Zhang > > > Subject: Re: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE > devices > > > using the storvsc driver > > > > > > Thanks, this looks much cleaner than the initial variant. > > > > > > > + if (dev_is_ide) { > > > > + storvsc_get_ide_info(device, &target, &path); > > > > + host_dev->path = device_info.path_id; > > > > + host_dev->target = device_info.target_id; > > > > + } else { > > > > + host_dev->path = device_info.path_id; > > > > + host_dev->target = device_info.target_id; > > > > + } > > > > > > Is using the device_info values in both branches intentional? If so > > > there's no need to have these assignments duplicated. > > > > While we set the values in both the branches, the value set is different; > > The IDE side encodes the bits differently and is appropriately parsed in the > > function storvsc_get_ide_info(). > > Is think that what Christoph meant was simplifying it to: > > if (dev_is_ide) > storvsc_get_ide_info(device, &target, &path); > > host_dev->path = device_info.path_id; > host_dev->target = device_info.target_id; Good point! It is still too early in the morning for me. Greg, if it is ok with you, in the interest of keeping the patch cops happy, I can submit a patch for this on top of this series. Regards, K. Y > > -- > > Sasha. > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel