Re: [PATCH v3] i2c: add support for Zhaoxin I2C controller

[Krzysztof Kozlowski <krzk@kernel.org>]

Your email is corrupted. It was sent with "Reply to:
0489dec4-cd7a-4f50-e811-d4798d514fb4@kernel.org":

Reply-To: <0489dec4-cd7a-4f50-e811-d4798d514fb4@kernel.org>
In-Reply-To: <0489dec4-cd7a-4f50-e811-d4798d514fb4@kernel.org>


On 07/06/2023 14:33, Hans Hu wrote:
>>> +
>>> +static int zxi2c_parse_resources(struct zxi2c *i2c)
>>> +{
>>> +	struct platform_device *pdev = to_platform_device(i2c->dev);
>>> +	struct resource *res;
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	i2c->regs = devm_ioremap_resource(&pdev->dev, res);
>> There is a helper for these two.
> 
> 
> I want to use the lower 8 bits of the physical address to
> distinguish the controller's index, to set reset_bitmask.

OK

> 
> 
>>> +	if (IS_ERR(i2c->regs))
>>> +		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->regs),
>>> +				"get base addr failed\n");
>>> +
>>> +	if (res->start & 0x20)
>>> +		i2c->reset_bitmask = BIT(4);
>>> +	else
>>> +		i2c->reset_bitmask = BIT(5);
>>> +
>>> +	i2c->irq = platform_get_irq(pdev, 0);
>>> +	if (i2c->irq < 0)
>>> +		return dev_err_probe(&pdev->dev, i2c->irq,
>>> +				"get irq failed\n");
>>> +
>>> +	zxi2c_get_bus_speed(i2c);
>>> +
>>> +	i2c->hrv = get_reversion(i2c->regs);
>>> +
>>> +	i2c->dynamic = get_dynamic_clock(i2c->regs);
>>> +	set_dynamic_clock(i2c->regs, i2c->dynamic);
>>> +
>>> +	i2c->fstp = get_fstp_value(i2c->regs);
>>> +	zxi2c_set_bus_speed(i2c);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int zxi2c_probe(struct platform_device *pdev)
>>> +{
>>> +	int error;
>>> +	struct zxi2c *i2c;
>>> +	struct pci_dev *pci;
>>> +	struct device *dev;
>>> +
>>> +	/* make sure this is zhaoxin platform */
>> Why? How can you match to different platform?
>>
> 
> In case a platform device named "IIC1D17" is also reported on
> non-zhaoxin platform, this driver may also be registered successfully,
> even if it shouldn't. In theory.

Why shouldn't it be registered on other platform? It's the same device,
isn't it?


Best regards,
Krzysztof