Re: [PATCH V7 3/7] ata: ahci_tegra: Update initialization sequence

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]

On Mon, Feb 12, 2018 at 10:56:42PM +0530, Preetham Chandru Ramchandra wrote:
> From: Preetham Ramchandra <pchandru@nvidia.com>
> 
> Update the controller initialization sequence and move
> t124 specifics to tegra124_ahci_init.
> 
> Signed-off-by: Preetham Chandru R <pchandru@nvidia.com>
> ---
> v7:
> * moveed tegra124_ahci_soc_data definition to be
>   just above the of_device_id table.
> ---
>  drivers/ata/ahci_tegra.c | 288 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 223 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> index 3a62eb246d80..7ffe8a97447a 100644
> --- a/drivers/ata/ahci_tegra.c
> +++ b/drivers/ata/ahci_tegra.c
[...]
> @@ -99,6 +159,14 @@ static const struct sata_pad_calibration tegra124_pad_calibration[] = {
>  	{0x14, 0x0e, 0x1a, 0x0e},
>  };
>  
> +struct tegra_ahci_ops {
> +	int (*init)(struct ahci_host_priv *hpriv);
> +};
> +
> +struct tegra_ahci_soc {
> +	struct tegra_ahci_ops	ops;
> +};

Just noticed this: it's slightly more customary to make ops a pointer to
a function table, like so:

	struct tegra_ahci_soc {
		const struct tegra_ahci_ops *ops;
	};

and then have:

	static const struct tegra_ahci_ops tegra124_ahci_ops = {
		.init = tegra124_ahci_init,
	};

	static const struct tegra_ahci_soc tegra124_ahci_soc = {
		.ops = &tegra124_ahci_ops,
	};

That's not terribly useful in this case because the ops are not passed
around or shared between multiple instances, so there are no benefits.
Either way is fine here, I guess.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]