> -----Original Message-----
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Friday, January 22, 2016 5:08 AM
> To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com;
> martin.peter...@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com;
> 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:the...@redhat.com]
> >> Sent: Wednesday, January 20, 2016 5:41 AM
> >> To: Raghava Aditya Renukunta;
> james.bottom...@hansenpartnership.com;
> >> martin.peter...@oracle.com; linux-scsi@vger.kernel.org
> >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> >> sierra.com; Scott Benesh; jthumsh...@suse.de;
> shane.seym...@hpe.com;
> >> 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.renuku...@pmcs.com>
> >>>
> >>> 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.renuku...@pmcs.com>
> >>> Reviewed-by: Shane Seymour <shane.seym...@hpe.com>
> >> 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 majord...@vger.kernel.org
> > 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
