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