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