Re: [PATCH 03/25] scsi: hisi_sas: add initial bare driver

[Xinwei Kong <kong.kongxinwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>]

On 2015/10/15 17:23, John Garry wrote:
> On 15/10/2015 09:49, Xinwei Kong wrote:
>>
>>
>> On 2015/10/12 23:20, John Garry wrote:
>>> This patch adds the initial bare driver for the HiSilicon
>>> SAS HBA. The driver includes no HW interaction, but only
>>> the changes to build and load the driver module.
>>>
>>> The HBA is a platform device.
>>>
>>> Signed-off-by: John Garry <john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>> ---
>>>   drivers/scsi/Kconfig                  |  1 +
>>>   drivers/scsi/Makefile                 |  1 +
>>>   drivers/scsi/hisi_sas/Kconfig         |  5 +++
>>>   drivers/scsi/hisi_sas/Makefile        |  2 ++
>>>   drivers/scsi/hisi_sas/hisi_sas.h      | 24 +++++++++++++++
>>>   drivers/scsi/hisi_sas/hisi_sas_init.c | 58 
>>> +++++++++++++++++++++++++++++++++++
>>>   6 files changed, 91 insertions(+)
>>>   create mode 100644 drivers/scsi/hisi_sas/Kconfig
>>>   create mode 100644 drivers/scsi/hisi_sas/Makefile
>>>   create mode 100644 drivers/scsi/hisi_sas/hisi_sas.h
>>>   create mode 100644 drivers/scsi/hisi_sas/hisi_sas_init.c
>>>
>>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>>> index 95f7a76..5c345f9 100644
>>> --- a/drivers/scsi/Kconfig
>>> +++ b/drivers/scsi/Kconfig
>>> @@ -1774,5 +1774,6 @@ source "drivers/scsi/pcmcia/Kconfig"
>>>   source "drivers/scsi/device_handler/Kconfig"
>>>
>>>   source "drivers/scsi/osd/Kconfig"
>>> +source "drivers/scsi/hisi_sas/Kconfig"
>>>
>>>   endmenu
>>> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
>>> index 1a8c9b5..03c30de 100644
>>> --- a/drivers/scsi/Makefile
>>> +++ b/drivers/scsi/Makefile
>>> @@ -158,6 +158,7 @@ obj-$(CONFIG_CHR_DEV_SCH)    += ch.o
>>>   obj-$(CONFIG_SCSI_ENCLOSURE)    += ses.o
>>>
>>>   obj-$(CONFIG_SCSI_OSD_INITIATOR) += osd/
>>> +obj-$(CONFIG_SCSI_HISI_SAS) += hisi_sas/
>>>
>>>   # This goes last, so that "real" scsi devices probe earlier
>>>   obj-$(CONFIG_SCSI_DEBUG)    += scsi_debug.o
>>> diff --git a/drivers/scsi/hisi_sas/Kconfig 
>>> b/drivers/scsi/hisi_sas/Kconfig
>>> new file mode 100644
>>> index 0000000..a7f47a2
>>> --- /dev/null
>>> +++ b/drivers/scsi/hisi_sas/Kconfig
>>> @@ -0,0 +1,5 @@
>>> +config SCSI_HISI_SAS
>>> +    tristate "HiSilicon SAS"
>>> +    select SCSI_SAS_LIBSAS
>>> +    help
>>> +        This driver supports HiSilicon's SAS HBA
>>> diff --git a/drivers/scsi/hisi_sas/Makefile 
>>> b/drivers/scsi/hisi_sas/Makefile
>>> new file mode 100644
>>> index 0000000..63c3c4d
>>> --- /dev/null
>>> +++ b/drivers/scsi/hisi_sas/Makefile
>>> @@ -0,0 +1,2 @@
>>> +obj-$(CONFIG_SCSI_HISI_SAS) += hisi_sas.o
>>> +hisi_sas-y+= hisi_sas_init.o
>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas.h 
>>> b/drivers/scsi/hisi_sas/hisi_sas.h
>>> new file mode 100644
>>> index 0000000..50204a2
>>> --- /dev/null
>>> +++ b/drivers/scsi/hisi_sas/hisi_sas.h
>>> @@ -0,0 +1,24 @@
>>> +/*
>>> + * Copyright (c) 2015 Linaro Ltd.
>>> + * Copyright (c) 2015 Hisilicon Limited.
>>> + *
>>> + * This program is free software; you can redistribute it and/or 
>>> modify
>>> + * it under the terms of the GNU General Public License as 
>>> published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + */
>>> +
>>> +#ifndef _HISI_SAS_H_
>>> +#define _HISI_SAS_H_
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/platform_device.h>
>>> +#include <scsi/libsas.h>
>>> +
>> why place some "include" head in .h file rather than .c file?
> This is private header within the module, which:
> - makes the code more concise
> - relocate functions within c files is cleaner
> - easier to change the kernel APIs we use in the module
>
this  .h file  wil be included by  "hisi_sas_main.c"   and 
"hisi_sas_init.c" file.
if all "include" term can't be used in both  ".c" file.  When you build 
this code,
it will add  some burden work.

Thank you
xinwei
>>> +#define DRV_NAME "hisi_sas"
>>> +#define DRV_VERSION "v1.0"
>>> +
>>> +#endif
>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_init.c 
>>> b/drivers/scsi/hisi_sas/hisi_sas_init.c
>>> new file mode 100644
>>> index 0000000..dd83430
>>> --- /dev/null
>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_init.c
>>> @@ -0,0 +1,58 @@
>>> +/*
>>> + * Copyright (c) 2015 Linaro Ltd.
>>> + * Copyright (c) 2015 Hisilicon Limited.
>>> + *
>>> + * This program is free software; you can redistribute it and/or 
>>> modify
>>> + * it under the terms of the GNU General Public License as 
>>> published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + */
>>> +
>>> +#include "hisi_sas.h"
>>> +
>>> +static const struct of_device_id sas_of_match[] = {
>>> +    { .compatible = "hisilicon,sas-controller-v1",},
>>> +    {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sas_of_match);
>>> +static int hisi_sas_probe(struct platform_device *pdev)
>>> +{
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int hisi_sas_remove(struct platform_device *pdev)
>>> +{
>>> +    return 0;
>>> +}
>>> +
>>> +static struct platform_driver hisi_sas_driver = {
>>> +    .probe = hisi_sas_probe,
>>> +    .remove = hisi_sas_remove,
>>> +    .driver = {
>>> +        .name = DRV_NAME,
>>> +        .of_match_table = sas_of_match,
>>> +    },
>>> +};
>>> +
>>> +static __init int hisi_sas_init(void)
>>> +{
>>> +    pr_info("hisi_sas: driver version %s\n", DRV_VERSION);
>>> +
>>> +    return platform_driver_register(&hisi_sas_driver);
>>> +}
>>> +
>>> +static __exit void hisi_sas_exit(void)
>>> +{
>>> +    platform_driver_unregister(&hisi_sas_driver);
>>> +}
>>> +
>>> +module_init(hisi_sas_init);
>>> +module_exit(hisi_sas_exit);
>>> +
>>> +MODULE_VERSION(DRV_VERSION);
>>> +MODULE_LICENSE("GPL");
>> V2
> Can add. We do say in the header that it is v2.
>
>>> +MODULE_AUTHOR("John Garry <john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>");
>>> +MODULE_DESCRIPTION("HISILICON SAS controller driver");
>>> +MODULE_ALIAS("platform:" DRV_NAME);
>>>
>>
>>
>> .
>>
>
> thanks,
> John
>
>
>
> .
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html