Re: [PATCH 12/25] scsi: hisi_sas: add v1 HW initialisation code

John Garry
2015-10-13T12:44:26Z

On 12/10/2015 19:46, Arnd Bergmann wrote:
> On Monday 12 October 2015 23:20:24 John Garry wrote:
>> @@ -387,6 +392,21 @@ static int hisi_sas_probe(struct platform_device *pdev)
>> }
>>
>> hisi_sas_init_add(hisi_hba);
>> +
>> + rc = hw_init_v1_hw(hisi_hba);
>> + if (rc)
>> + goto err_out_ha;
>> +
>> + rc = interrupt_init_v1_hw(hisi_hba);
>> + if (rc)
>> + goto err_out_ha;
>> +
>> + rc = interrupt_openall_v1_hw(hisi_hba);
>> + if (rc)
>> + goto err_out_ha;
>> +
>> + phys_init_v1_hw(hisi_hba);
>> +
>> rc = scsi_add_host(shost, &pdev->dev);
>> if (rc)
>> goto err_out_ha;
>>
>
> As the probe function matches against the "hisilicon,sas-controller-v1"
> compatible string and contains mostly code that is specific to v1, I
> think it would be cleaner to move that to the hisi_sas_v1_hw.c as well
> and make the functions above static but make the common functions
> called here (hisi_sas_init_add etc) global.
>
> That would also include the hisi_sas_driver structure.
>
> Arnd
>
> .
>
Hello,

Just to be clear, are you saying that you would prefer hisi_sas_probe()
and struct hisi_sas_driver to be relocated to hisi_sas_v1_hw.c?

I wanted to keep hisi_sas_v1_hw.c containing only code which accesses HW.

I could consoldate the calls of hw_init_v1_hw(), interrupt_init_v1_hw(),
interrupt_openall_v1_hw(), and phys_init_v1_hw() to a single function hw
init function call. Then most of the code in the probe function will not
be specific to v1.

Regards,
John