Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string

[Santosh Shilimkar <santosh.shilimkar@ti.com>]

On Friday 23 August 2013 01:09 PM, Sekhar Nori wrote:
> 
> 
> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
>>> On 23.08.2013 16:23, Santosh Shilimkar wrote:
>>>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>>>
>>>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>>>> +	{
>>>>> +		.compatible	= "ti,am3352-cpsw",
>>>>
>>>> I didn't notice this earlier, but can't you use the IP version
>>>> as a compatible instead of using a SOC name. Whats really SOC specific
>>>> on this IP ? Sorry i have missed any earlier discussion on this but
>>>> this approach doesn't seem good. Its like adding SOC checks in the
>>>> driver subsystem.
>>>
>>> As I already mentioned in the cover letter and in the commit message, I
>>> just don't know which criteria makes most sense here.
>>>
>>> On a general note, I would say that chances that this exactly IP core
>>> with the same version number will appear on some other silicon which
>>> doesn't support the control mode register in an AM33xx fashion, is not
>>> necessarily negligible.
>>>
>>> So what that new compatible string denotes is the cpsw in a version as
>>> found on am3352 SoCs, which is actually exactly what it does.
>>>
>>> I don't have a strong opinion here, but see your point. I just don't
>>> have a better idea on how to treat that.
>>>
>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
> 
> If this could be handled using IP version then the right way would be to
> just read the IP version from hardware and use it. No need of DT property.
> 
Thats fine as well but I thought the patch needed additional properties like
CM reg-address come from DT and hence the separate compatible. If you can
manage without that, thats even better.

Regards,
Santosh