Re: [PATCH 02/12] mfd: intel-m10-bmc: Create m10bmc_platform_info for type specific info

From: Xu Yilun
Date: Fri Nov 11 2022 - 05:23:32 EST


On 2022-11-08 at 16:42:55 +0200, Ilpo Järvinen wrote:
> BMC type specific info is currently set by a switch/case block. The
> size of this info is expected to grow as more dev types and features
> are added which would have made the switch block bloaty.
>
> Store type specific info into struct and place them into .driver_data
> instead because it makes things a bit cleaner.
>
> Reviewed-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> drivers/mfd/intel-m10-bmc.c | 50 +++++++++++++++++--------------
> include/linux/mfd/intel-m10-bmc.h | 14 +++++++++
> 2 files changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
> index ee167c5dcd29..762808906380 100644
> --- a/drivers/mfd/intel-m10-bmc.c
> +++ b/drivers/mfd/intel-m10-bmc.c
> @@ -156,15 +156,17 @@ static int check_m10bmc_version(struct intel_m10bmc *ddata)
> static int intel_m10_bmc_spi_probe(struct spi_device *spi)
> {
> const struct spi_device_id *id = spi_get_device_id(spi);
> + const struct intel_m10bmc_platform_info *info;
> struct device *dev = &spi->dev;
> - struct mfd_cell *cells;
> struct intel_m10bmc *ddata;
> - int ret, n_cell;
> + int ret;
>
> ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> if (!ddata)
> return -ENOMEM;
>
> + info = (struct intel_m10bmc_platform_info *)id->driver_data;
> + ddata->info = info;

Where to use the ddata->info?

> ddata->dev = dev;
>
> ddata->regmap =
> @@ -183,24 +185,8 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
> return ret;
> }
>
> - switch (id->driver_data) {
> - case M10_N3000:
> - cells = m10bmc_pacn3000_subdevs;
> - n_cell = ARRAY_SIZE(m10bmc_pacn3000_subdevs);
> - break;
> - case M10_D5005:
> - cells = m10bmc_d5005_subdevs;
> - n_cell = ARRAY_SIZE(m10bmc_d5005_subdevs);
> - break;
> - case M10_N5010:
> - cells = m10bmc_n5010_subdevs;
> - n_cell = ARRAY_SIZE(m10bmc_n5010_subdevs);
> - break;
> - default:
> - return -ENODEV;
> - }
> -
> - ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, n_cell,
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
> + info->cells, info->n_cells,
> NULL, 0, NULL);
> if (ret)
> dev_err(dev, "Failed to register sub-devices: %d\n", ret);
> @@ -208,10 +194,28 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
> return ret;
> }
>
> +static const struct intel_m10bmc_platform_info m10bmc_m10_n3000 = {
> + .type = M10_N3000,

Is the type enum still useful? Found no usage.

Thanks,
Yilun

> + .cells = m10bmc_pacn3000_subdevs,
> + .n_cells = ARRAY_SIZE(m10bmc_pacn3000_subdevs),
> +};
> +
> +static const struct intel_m10bmc_platform_info m10bmc_m10_d5005 = {
> + .type = M10_D5005,
> + .cells = m10bmc_d5005_subdevs,
> + .n_cells = ARRAY_SIZE(m10bmc_d5005_subdevs),
> +};
> +
> +static const struct intel_m10bmc_platform_info m10bmc_m10_n5010 = {
> + .type = M10_N5010,
> + .cells = m10bmc_n5010_subdevs,
> + .n_cells = ARRAY_SIZE(m10bmc_n5010_subdevs),
> +};
> +
> static const struct spi_device_id m10bmc_spi_id[] = {
> - { "m10-n3000", M10_N3000 },
> - { "m10-d5005", M10_D5005 },
> - { "m10-n5010", M10_N5010 },
> + { "m10-n3000", (kernel_ulong_t)&m10bmc_m10_n3000 },
> + { "m10-d5005", (kernel_ulong_t)&m10bmc_m10_d5005 },
> + { "m10-n5010", (kernel_ulong_t)&m10bmc_m10_n5010 },
> { }
> };
> MODULE_DEVICE_TABLE(spi, m10bmc_spi_id);
> diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
> index d77383a8a410..e58115f514b9 100644
> --- a/include/linux/mfd/intel-m10-bmc.h
> +++ b/include/linux/mfd/intel-m10-bmc.h
> @@ -124,14 +124,28 @@ enum m10bmc_type {
> /* Address of 4KB inverted bit vector containing staging area FLASH count */
> #define STAGING_FLASH_COUNT 0x17ffb000
>
> +/**
> + * struct intel_m10bmc_platform_info - Intel MAX 10 BMC platform specific information
> + * @type: the type of MAX10 BMC
> + * @cells: MFD cells
> + * @n_cells: MFD cells ARRAY_SIZE()
> + */
> +struct intel_m10bmc_platform_info {
> + enum m10bmc_type type;
> + struct mfd_cell *cells;
> + int n_cells;
> +};
> +
> /**
> * struct intel_m10bmc - Intel MAX 10 BMC parent driver data structure
> * @dev: this device
> * @regmap: the regmap used to access registers by m10bmc itself
> + * @info: the platform information for MAX10 BMC
> */
> struct intel_m10bmc {
> struct device *dev;
> struct regmap *regmap;
> + const struct intel_m10bmc_platform_info *info;
> };
>
> /*
> --
> 2.30.2
>