Re: [PATCH v2 1/2] phy: dw-dphy-rx: Add Synopsys DesignWare D-PHY RX

[Krzysztof Kozlowski <krzk@kernel.org>]

On Wed, Jul 09, 2025 at 07:42:20PM -0700, Karthik Poduval wrote:
> +static const struct regmap_config dw_dphy_regmap_cfg1 = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.name = "dw-dhpy-cfg1",
> +	.fast_io = true,
> +};
> +
> +/**
> + * dw_dphy_regmap_cfg2 - Register map configuration for DW DPHY
> + * @reg_bits: Width of register address in bits (32)
> + * @val_bits: Width of register value in bits (32)
> + * @reg_stride: Number of bytes between registers (4)
> + * @name: Name identifier for this register map
> + * @fast_io: Flag to indicate fast I/O operations are supported
> + *
> + **/

Drop

> +static const struct regmap_config dw_dphy_regmap_cfg2 = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.name = "dw-dhpy-cfg2",
> +	.fast_io = true,
> +};
> +
> +/**
> + * dw_dphy_probe - Probe and initialize DW DPHY device
> + * @pdev: Platform device pointer
> + * Return: 0 on success, negative error code on failure
> + *
> + **/

Drop

> +static int dw_dphy_probe(struct platform_device *pdev)
> +{
> +	struct dw_dphy *dphy;
> +	struct resource *res;
> +	struct device *dev = &pdev->dev;
> +	struct phy_provider *phy_provider;
> +	int ret;
> +
> +	dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
> +	if (!dphy)
> +		return -ENOMEM;
> +
> +	dphy->dt_data =
> +		(struct dt_data_dw_dphy *)of_device_get_match_data(&pdev->dev);
> +	dev_set_drvdata(&pdev->dev, dphy);
> +	dphy->dev = &pdev->dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	dphy->iomem_cfg1 = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(dphy->iomem_cfg1))
> +		return PTR_ERR(dphy->iomem_cfg1);
> +
> +	dphy->regmap_cfg1 =
> +		devm_regmap_init_mmio(dev, dphy->iomem_cfg1, &dw_dphy_regmap_cfg1);
> +	if (IS_ERR(dphy->regmap_cfg1))
> +		return PTR_ERR(dphy->regmap_cfg1);
> +
> +	ret = devm_regmap_field_bulk_alloc(dev, dphy->regmap_cfg1, dphy->rf_cfg1,
> +					   dw_dphy_v1_2_cfg1, DW_DPHY_RF_CFG1_MAX);
> +	if (ret < 0) {
> +		dev_err(dev, "Could not alloc RF\n");
> +		return ret;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	dphy->iomem_cfg2 = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(dphy->iomem_cfg2))
> +		return PTR_ERR(dphy->iomem_cfg2);
> +
> +	dphy->regmap_cfg2 = devm_regmap_init_mmio(dev, dphy->iomem_cfg2,
> +						 &dw_dphy_regmap_cfg2);
> +	if (IS_ERR(dphy->regmap_cfg2))
> +		return PTR_ERR(dphy->regmap_cfg2);
> +
> +	ret = devm_regmap_field_bulk_alloc(dev, dphy->regmap_cfg2, dphy->rf_cfg2,
> +					   dw_dphy_v1_2_cfg2, DW_DPHY_RF_CFG2_MAX);
> +	if (ret < 0) {
> +		dev_err(dev, "Could not alloc RF\n");
> +		return ret;
> +	}
> +
> +	dphy->phy = devm_phy_create(&pdev->dev, NULL, dphy->dt_data->phy_ops);
> +	if (IS_ERR(dphy->phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		return PTR_ERR(dphy->phy);
> +	}
> +
> +	phy_set_drvdata(dphy->phy, dphy);
> +	phy_provider =
> +		devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +/**
> + * dw_dphy_of_match - Device tree match table for DW DPHY
> + * @compatible: Compatible string to match device tree node
> + * @data: Pointer to configuration data for matched device
> + *
> + * Table of compatible strings and associated configuration data
> + * for supported DW DPHY variants.
> + * Currently supports:
> + * - DW DPHY v1.2 ("snps,dw-dphy-1p2")
> + *
> + **/

Drop

> +static const struct of_device_id dw_dphy_of_match[] = {
> +	{ .compatible = "snps,dw-dphy-1p2", .data = &dw_dphy_1p2 },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, dw_dphy_of_match);
> +
> +/**
> + * dw_dphy_platform_driver - Platform driver structure for DW DPHY
> + * @probe: Pointer to probe function called on device discovery
> + * @driver: Core driver structure containing:
> + *         - name: Driver name used for matching and debugging
> + *         - of_match_table: Table of compatible device tree matches
> + *
> + **/

Drop all such useless generic kerneldocs. Not helpful. Keep useful ones,
so ones not saying obvious parts of core


> +static struct platform_driver dw_dphy_platform_driver = {
> +	.probe		= dw_dphy_probe,

Best regards,
Krzysztof