> -----Original Message----- > From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] > Sent: Friday, January 22, 2016 5:08 AM > To: Raghava Aditya Renukunta; James.Bottomley@xxxxxxxxxxxxxxxxxxxxx; > martin.petersen@xxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx > Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- > sierra.com; Scott Benesh; jthumshirn@xxxxxxx; shane.seymour@xxxxxxx; > zzzDavid Carroll > Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization > > On 20.1.2016 21:43, Raghava Aditya Renukunta wrote: > > Hello Tomas, > > > >> -----Original Message----- > >> From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] > >> Sent: Wednesday, January 20, 2016 5:41 AM > >> To: Raghava Aditya Renukunta; > James.Bottomley@xxxxxxxxxxxxxxxxxxxxx; > >> martin.petersen@xxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx > >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- > >> sierra.com; Scott Benesh; jthumshirn@xxxxxxx; > shane.seymour@xxxxxxx; > >> zzzDavid Carroll > >> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization > >> > >> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote: > >>> From: Raghava Aditya Renukunta > <raghavaaditya.renukunta@xxxxxxxx> > >>> > >>> During EEH PCI hotplug activity kernel unloads and loads the driver, > >>> causing character device to be unregistered(aac_remove_one).When > the > >>> driver is loaded back using aac_probe_one the character device needs > >>> to be registered again for the AIF management tools to work. > >>> > >>> Fixed by adding code to register character device in aac_probe_one if > >>> it is unregistered in aac_remove_one. > >>> > >>> Changes in V2: > >>> Added macros to track character device state > >>> > >>> Changes in V3: > >>> None > >>> > >>> Signed-off-by: Raghava Aditya Renukunta > >> <raghavaaditya.renukunta@xxxxxxxx> > >>> Reviewed-by: Shane Seymour <shane.seymour@xxxxxxx> > >> Hi Raghava, > >> when aacraid is loaded (modprobe) without an controller attached to the > >> system > >> the driver loads and creates the character device. Later when you hotplug > a > >> device and remove again we see the driver loaded but now without the > >> char device. I'd prefer consistency here - either create the char device > >> when the first controller is probed (preferred) or do not remove it > >> until the driver exits. > >> This is not a nack, just a wish that you changed it in next series. > >> > >> --tm > > > > Yes I will make the necessary changes so that character device is created > when > > The controller is probed, and when the driver is removed > (aac_remove_one),delete > > the character device. I will keep the character device during resume and > suspend. > > > > Do you want to do this in the next version of the patches or the next series > of patches after this one is > > Accepted. ? > > sure, next series is fine, as I wrote already Will do , Thank you Tomas. > > > > Regards, > > Raghava Aditya > > > > > >>> --- > >>> drivers/scsi/aacraid/aacraid.h | 7 +++++++ > >>> drivers/scsi/aacraid/linit.c | 21 ++++++++++++++------- > >>> 2 files changed, 21 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h > >>> index 3473668..4b669ef 100644 > >>> --- a/drivers/scsi/aacraid/aacraid.h > >>> +++ b/drivers/scsi/aacraid/aacraid.h > >>> @@ -94,6 +94,13 @@ enum { > >>> #define aac_phys_to_logical(x) ((x)+1) > >>> #define aac_logical_to_phys(x) ((x)?(x)-1:0) > >>> > >>> +/* > >>> + * These macros are for keeping track of > >>> + * character device state. > >>> + */ > >>> +#define AAC_CHARDEV_UNREGISTERED (-1) > >>> +#define AAC_CHARDEV_NEEDS_REINIT (-2) > >>> + > >>> /* #define AAC_DETAILED_STATUS_INFO */ > >>> > >>> struct diskparm > >>> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c > >>> index 27b3fcd..057c07c 100644 > >>> --- a/drivers/scsi/aacraid/linit.c > >>> +++ b/drivers/scsi/aacraid/linit.c > >>> @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION); > >>> > >>> static DEFINE_MUTEX(aac_mutex); > >>> static LIST_HEAD(aac_devices); > >>> -static int aac_cfg_major = -1; > >>> +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED; > >>> char aac_driver_version[] = AAC_DRIVER_FULL_VERSION; > >>> > >>> /* > >>> @@ -1125,6 +1125,13 @@ static void __aac_shutdown(struct aac_dev * > >> aac) > >>> else if (aac->max_msix > 1) > >>> pci_disable_msix(aac->pdev); > >>> } > >>> +static void aac_init_char(void) > >>> +{ > >>> + aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops); > >>> + if (aac_cfg_major < 0) { > >>> + pr_err("aacraid: unable to register \"aac\" device.\n"); > >>> + } > >>> +} > >>> > >>> static int aac_probe_one(struct pci_dev *pdev, const struct > pci_device_id > >> *id) > >>> { > >>> @@ -1182,6 +1189,9 @@ static int aac_probe_one(struct pci_dev *pdev, > >> const struct pci_device_id *id) > >>> shost->max_cmd_len = 16; > >>> shost->use_cmd_list = 1; > >>> > >>> + if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT) > >>> + aac_init_char(); > >>> + > >>> aac = (struct aac_dev *)shost->hostdata; > >>> aac->base_start = pci_resource_start(pdev, 0); > >>> aac->scsi_host_ptr = shost; > >>> @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev > >> *pdev) > >>> pci_disable_device(pdev); > >>> if (list_empty(&aac_devices)) { > >>> unregister_chrdev(aac_cfg_major, "aac"); > >>> - aac_cfg_major = -1; > >>> + aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT; > >>> } > >>> } > >>> > >>> @@ -1681,11 +1691,8 @@ static int __init aac_init(void) > >>> if (error < 0) > >>> return error; > >>> > >>> - aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops); > >>> - if (aac_cfg_major < 0) { > >>> - printk(KERN_WARNING > >>> - "aacraid: unable to register \"aac\" device.\n"); > >>> - } > >>> + aac_init_char(); > >>> + > >>> > >>> return 0; > >>> } > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
