Re: [PATCH 2/6] mailbox/omap: add support for parsing dt devices

[Suman Anna]

On 07/09/2014 03:29 AM, Tony Lindgren wrote:
> * Suman Anna <s-anna@xxxxxx> [140708 10:57]:
>> Hi Tony, Pavel,
>>
>> On 07/04/2014 03:23 AM, Tony Lindgren wrote:
>>> * Pavel Machek <pavel@xxxxxx> [140704 01:07]:
>>>> Hi!
>>>>
>>>>>>>>>>> The non-DT support has to be maintained for now to not break
>>>>>>>>>>> OMAP3 legacy boot, and the legacy-style code will be cleaned
>>>>>>>>>>> up once OMAP3 is also converted to DT-boot only.
>>>>>>>>>>
>>>>>>>>>>> @@ -587,24 +606,157 @@ static int omap_mbox_unregister(struct omap_mbox_device *mdev)
>>>>>>>>>>>  	return 0;
>>>>>>>>>>>  }
>>>>>>>>>>>  
>>>>>>>>>>> +static const struct omap_mbox_device_data omap2_data = {
>>>>>>>>>>> +	.num_users	= 4,
>>>>>>>>>>> +	.num_fifos	= 6,
>>>>>>>>>>> +	.intr_type	= MBOX_INTR_CFG_TYPE1,
>>>>>>>>>>> +};
>>>>>>>>>>> +
>>>>>>>>>>> +static const struct omap_mbox_device_data omap3_data = {
>>>>>>>>>>> +	.num_users	= 2,
>>>>>>>>>>> +	.num_fifos	= 2,
>>>>>>>>>>> +	.intr_type	= MBOX_INTR_CFG_TYPE1,
>>>>>>>>>>> +};
>>>>>>>>>>> +
>>>>>>>>>>> +static const struct omap_mbox_device_data am335x_data = {
>>>>>>>>>>> +	.num_users	= 4,
>>>>>>>>>>> +	.num_fifos	= 8,
>>>>>>>>>>> +	.intr_type	= MBOX_INTR_CFG_TYPE2,
>>>>>>>>>>> +};
>>>>>>>>>>
>>>>
>>>>>> Aha, ok, then the intr_type should be derived from
>>>>>> compatible-string. Or rather... you should have three
>>>>>> compatible-strings for the three possibilities? (And then subtype,
>>>>>> currently unused, in case there are more hw differences).
>>>>>
>>>>> The compatible string can and should be separate for each revision
>>>>> unless they are the same exacat hardware revision.
>>>>
>>>> ACK.
>>
>> I checked the revision register from all SoCs. OMAP2 and OMAP3 have
>> different revisions compared to OMAP4+. All of OMAP4, OMAP5, DRA7,
>> AM335x and AM437x have the same version, but with different num-fifos
>> and num-users.  So, I can switch back to using omap4-mailbox for all of
>> these SoCs only if we encode the num-fifos and num-users in DT.
>>
>>>>
>>>>>>> two are HW IP design parameters, so in general putting them in DT isn't
>>>>>>> completely a bad idea, but I will wait to see if there are any further
>>>>>>> comments on this from Tony or DT maintainers before I make changes.
>>>>>>
>>>>>> Ok, right... I'd vote for putting them into DT.
>>>>>
>>>>> I would avoid adding custom DT properties where possible and let the
>>>>> driver just initialize the right data based on the compatible flag.
>>>>
>>>> If these are HW IP design parameters, we can expect to see many
>>>> different combinations. Yet we know ahead of time how to handle
>>>> different parameters HW people select.
>>
>> That's right, the above OMAP4+ SoCs already demonstrate this behavior.
>>
>>>>
>>>> Thus IMO we should do it in the device tree.
>>>
>>> Oh you mean from supporting new hardware with just .dts changes?
>>> From that point of view it makes sense to have them as DT properties,
>>> so I'm fine with that.
>>>
>>> Let's just try to use something that's generic like fifosize. No idea
>>> how the property for num_fifos should be handled though as that
>>> implies some knowledge in the driver which num_users have fifos?
>>
>> The fifos are not per num_users, but rather the total number of fifos
>> within the IP block. The num_users will be the number of targets the IP
>> block can interrupt. I tried looking for generic properties, but there
>> weren't any that seem to fit the description. If you want generic names,
>> I can use num-fifos and num-users, otherwise will stick to the
>> names defined in the previous series.
> 
> OK since we already have some .dts entries with ti,mbox-num-fifos and
> ti,mbox-num-users I'd use those for now. Adding parsing for a generic
> property can be done later on.

Alright, will stick to the existing properties.

> 
>>> So unless that can be described clearly in a DT property as well,
>>> the binding might be still unusable for new hardware :)
>>>
>>
>> I don't expect the OMAP mailbox IP to change much in the future. There
>> is a FIFO depth parameter as well, but that's constant in all the
>> current versions, and even if they change it, I can already use the
>> generic property for that.
> 
> OK
>  
>> Tony,
>> Depending on the agreement here, I may have to respin the OMAP
>> mailbox DT/hwmod cleanup series [1]
> 
> OK let me know.

I will refresh both this series and the mailbox DT/hwmod cleanup with
these changes, and post the patches tomorrow.

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/