On Aug 12, 2013, at 11:59 AM, Tomasz Figa wrote:
> On Monday 12 of August 2013 17:43:54 Mark Rutland wrote:
>> [Adding other devicetree maintainers to Cc]
>>
>> On Mon, Aug 12, 2013 at 01:56:07PM +0100, Lu Jingchang-B35083 wrote:
>>> ________________________________________
>>>
>>>> 发件人: Mark Rutland [mark.rutland@xxxxxxx]
>>>> 发送时间: 2013年8月10日 22:08
>>>> 收件人: Lu Jingchang-B35083
>>>> 抄送: wsa@xxxxxxxxxxxxx; Estevam Fabio-R49496; Li Xiaochun-B41219;
>>>> s.hauer@xxxxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; Jin
>>>> Zhengxiong-R64188; shawn.guo@xxxxxxxxxx;
>>>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx 主题: Re: [PATCH v3 2/2] i2c:
>>>> imx: Add Vybrid VF610 I2C controller support> >
>>>> On Fri, Aug 02, 2013 at 05:44:08AM +0100, Jingchang Lu wrote:
>>>>> Add Freescale Vybrid VF610 I2C controller support to
>>>>>
>>>>> imx I2C driver framework.
>>>>>
>>>>> Some operation is different from imx I2C controller.
>>>>>
>>>>> The register offset, the i2c clock divider value table,
>>>>> the module enabling(I2CR_IEN) which is just invert with imx,
>>>>> and the interrupt flag(I2SR) clearing opcode is w1c on VF610
>>>>> but w0c on imx.
>>>>>
>>>>> Signed-off-by: Jason Jin <Jason.jin@xxxxxxxxxxxxx>
>>>>> Signed-off-by: Xiaochun Li <b41219@xxxxxxxxxxxxx>
>>>>> Signed-off-by: Jingchang Lu <b35083@xxxxxxxxxxxxx>
>>>>> ---
>>>>>
>>>>> changes in v3:
>>>>> Using struct naming the i2c clock {div, regval} pair.
>>>>>
>>>>> Using address shift handling registers address difference.
>>>>>
>>>>> changes in v2:
>>>>> Fix building section mismatch(es) warning.
>>>>>
>>>>> drivers/i2c/busses/i2c-imx.c | 146
>>>>> ++++++++++++++++++++++++++++++++++++------- 1 file changed, 122
>>>>> insertions(+), 24 deletions(-)
>>>>
>>>> [...]
>>>>
>>>>> @@ -145,6 +233,7 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);
>>>>>
>>>>> static const struct of_device_id i2c_imx_dt_ids[] = {
>>>>>
>>>>> { .compatible = "fsl,imx1-i2c", .data =
>>>>> &imx_i2c_devtype[IMX1_I2C], },
>>>>> { .compatible = "fsl,imx21-i2c", .data =
>>>>> &imx_i2c_devtype[IMX21_I2C], },> >>
>>>>> + { .compatible = "fsl,vf610-i2c", .data =
>>>>> &imx_i2c_devtype[VF610_I2C], },> >>
>>>>> { /* sentinel */ }
>>>>>
>>>>> };
>>>>
>>>> That string doesn't seem to be documented anywhere (from a quick grep
>>>> of Documentation/devicetree), and there's no binding update included
>>>> here. It would be nice for that to be fixed :)
>>>
>>> [Lu Jingchang]
>>> The binding string for i2c-imx driver in
>>> Documentation/devicetree/bindings/i2c/i2c-imx.txt use a wildcard
>>> format of "- compatible : Should be "fsl,<chip>-i2c" " for device
>>> using this driver. Neither fsl,imx1-i2c nor fsl,imx21-i2c is
>>> described in the binding document. So I just leave the vf610 i2c
>>> compatible with this.
>> I'm not a big fan on wildcards in bindings, as it leaves people free to
>> put anything in and claim it's a documented binding, and makes it far
>> harder for an os to actually implement drivers for said binding, as
>> there's no canonical reference for the set of valid variations.
>>
>> Obviously there is some precedent, but I'm not sure it's something we
>> want to stick with, and we can prevent it my updating the documentation
>> now.
>>
>> Does anyone else have an opinion?
>
> In case of Samsung platforms we decided to always use the name of first
> SoC in which given IP appeared and list all compatible SoCs in binding
> documentation. This is IMHO the most consistent way, as there is no
> confusion about IP versions (not always listed in documentation, not even
> saying about unavailable documentation) or potential problems with new
> SoCs matching the wildcard, but having different IP.
>
> In this particular case, the <chip> wildcard can be easily transformed
> into a non-wildcard binding, by listing all supported <chip> values, i.e.
> adding following text to binding documentation:
>
> 8<--
> The <chip> can be one of:
> - imx1 - for i.MX1 and compatible SoCs,
> - imx21 - for i.MX21 and compatible SoCs,
> - vf610 - for Vybrid VF610 and compatible SoCs.
> -->8
>
> Best regards,
> Tomasz
One way to handle this is to introduce a SoC Family <chip> name doc with the possible list of names and than reference that doc in the binding. Kinda like how we have vendor-prefixes.txt.
And maybe instead of doing <chip> its <imx-chip> or some reference that isn't too generic such that in the future a .dts could be validated.
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
