Hari, On Wed, Nov 24, 2010 at 18:31, Kanigeri, Hari <h-kanigeri2@xxxxxx> wrote: > On Wed, Nov 24, 2010 at 2:50 AM, Varadarajan, Charulatha <charu@xxxxxx> wrote: >> On Wed, Nov 24, 2010 at 13:52, Felipe Balbi <balbi@xxxxxx> wrote: >>> On Wed, Nov 24, 2010 at 10:46:04AM +0530, Varadarajan, Charulatha wrote: >>>>> >>>>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c >>>>> index 48e161c..a1c6bd9 100644 >>>>> --- a/arch/arm/plat-omap/mailbox.c >>>>> +++ b/arch/arm/plat-omap/mailbox.c >>>>> @@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct >>>>> omap_mbox **list) >>>>> ret = PTR_ERR(mbox->dev); >>>>> goto err_out; >>>>> } >>>>> + if (cpu_is_omap44xx()) >>>> >>>> Do not use cpu_is* checks in plat-omap/* >>> >>> see the previous thread. >> >> Referring to [1], I do not find why cpu_is* checks is used in plat-omap and >> why it can't be avoided. >> >> In [1], it was suggested to create the pdata field that will be >> populated at init >> time using the cpu_is* check. But in this version, I am finding that >> this is done >> in plat-omap. This can be handled in mach-omap layer itself and can be passed >> as a pdata field which can be extracted during probe. >> >> [1] https://patchwork.kernel.org/patch/337131/ >> > > Here are these reasons why I did this way. > > - The function omap_mbox_register is called only once during probe > time, and I thought it was ok to call cpu check once during probe > time. AFAIK it is not okay. Patches are not accepted by maintainers if cpu_is* checks are newly added in plat-omap/* Moreover, the plat-omap is for common code between omap1 and omap2+ If any specific info is required due to unavoidable reasons, they should be managed using pdata fld/ by some other means. > Since each mbox instance needs to be aware of the rev field this > was the right place to add since this function iterates through the > list during probe time. There are already calls to cpu_is_omap44xx in > mailbox probe function. Infact , there are some more drivers like that. But some of them are getting cleaned up slowly (work under progress). But old code having cpu_is* checks does not justify adding a new cpu_is* check. As I mentioned, it is repeatedly being insisted in LO mailing list to avoid cpu_is* checks in plat-omap and to clean up the drivers to avoid these checks. > > - platform data is not present for mailbox module. I could add it for > revision sake but I would prefer not to do that since this will be a > throw away code once the hwmod infrastructure is ready (note: mailbox > hwmod patches are under review), and amount of mailbox driver rework > might be considerable. I would leave this to Tony/ Felipe/ Benoit to decide on this. If agreed, I don't see any issue. But there should be atleast a "TODO" mentioning that this needs to be cleaned up. > > - I could wait till the hwmod patches are ready to include this > change, but don't want to put the dependency with hwmod since this is > a critical fix and want to make it available in the mainline kernel. > > Please let me know what you suggest. > > Thank you, > Best regards, > Hari Kanigeri > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
