On Thu, Nov 25, 2010 at 1:04 AM, Varadarajan, Charulatha <charu@xxxxxx> wrote: > 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. > fair enough. I am dropping this patch from the patch set and will send it with Omar's hwmod patches. I will send the revised patch set excluding this patch. Thank you, Best regards, Hari -- 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
