On Mon, Nov 02, 2009 at 08:13:21AM -0600, Nishanth Menon wrote:
> Alexander Shishkin had written, on 11/02/2009 07:12 AM, the following:
> >2009/10/21 Nishanth Menon <nm@xxxxxx>:
> >>Paul Walmsley had written, on 10/20/2009 06:14 PM, the following:
> >>>Hi Vikram, Nishanth, Richard,
> >>>
> >>>a few comments on this:
> >>>
> >>>On Tue, 20 Oct 2009, Vikram Pandita wrote:
> >>>
> >>>>Add bits for future expansion of omap_chip_id type field.
> >>>>This is needed to accomodate 3630ES1 chip id which is bit10
> >>>...
> >>>
> >>>>diff --git a/arch/arm/plat-omap/include/plat/cpu.h
> >>>>b/arch/arm/plat-omap/include/plat/cpu.h
> >>>>index 7cb0556..922bf1c 100644
> >>>>--- a/arch/arm/plat-omap/include/plat/cpu.h
> >>>>+++ b/arch/arm/plat-omap/include/plat/cpu.h
> >>>>@@ -45,7 +45,7 @@ int omap_type(void);
> >>>> struct omap_chip_id {
> >>>> u8 oc;
> >>>>- u8 type;
> >>>>+ u32 type;
> >>>> };
> >>>Just wanted to understand the motivation for using the u32.
> >>>Earlier in the life of these patches, two comments were mentioned: the
> >>>desire to 'futureproof' and the desire to reserve space for other
> >>>34xx-family parts.
> >>>
> >>>Regarding 'futureproofing:' that's part of the reason that a separate
> >>>struct was defined for this: to prevent code that uses it from depending on
> >>>the size of the type. (Originally it was a typedef, but Linus hates
> >>>typedefs...) So it shouldn't matter how big or small the type is here, as
> >>>long as it can handle all of the bits allocated for it.
> >>>
> >>>Also mentioned was the idea of reserving space for other 34xx-family
> >>>chips. I'd suggest simply renumbering the bits when and if those versions
> >>>appear. Code that uses the omap_chip_id system should always use the macros
> >>>(e.g. CHIP_IS_OMAP3430) and not encode separate bit shift values, so
> >>>renumbering should be completely safe and transparent for core code. Module
> >>>code shouldn't be using the omap_chip code, it's for core usage only.
> >>>
> >>>So, since only one bit is being added, why not continue the use of the u8?
> >>> Then when the next bits need to be added, the type can be expanded at that
> >>>point, and the bits renumbered if necessary. This should be a completely
> >>>transparent operation for code that uses it. Vikram's original patch:
> >>>
> >>> http://patchwork.kernel.org/patch/54847/
> >>>
> >>>should be fine.
> >>Assumptions:
> >>a) omap_chip_id is supposedly constant for all devices within the same
> >>family. 3630, 3430 rev x will belong to the same family.
> >If my understanding of the matter is correct, that's only possible if
> >you can foretell the total number of upcoming 34xx revisions worth
> >mentioning in the code. Also, can you please elaborate on why is it
> >supposed to be constant?
> The assumption I made was that omap_chip_id structure was to store
> info regarding the chip and used by the pm framework. The term
> constant, which I used, is probably wrong in this context, esp
> considering that we cannot foretell all the upcoming revisions of
> the 34xx family.
Ok, thanks for clarifying.
> >>Issues with the strategy of restricting to the current 8 bits:
> >>a) Why extrabits now:
> >>we have 8 bits now and we would have used all 8 bits with 3630 with the
> >>mentioned patch. What do we do with the next revision of 3430? Do we want to
> >>increase the size once it comes along? OR Do we want to do it right now? Why
> >>wait till we get additional silicons to go figure how to add those bits as
> >>Richard pointed out, when there could be one more in the pipeline?
> >But this code will have to be revisited for each additional silicon
> >revision anyway, right? Why reserve now?
> >
> Agreed, that is one of the possible approaches we could take (and
> seems to be the common consensus), we can review the structure at a
> later point of time.
>
> http://patchwork.kernel.org/patch/54847/ seems to be the right
> direction for now at least.
Ok, so let's have it in, perhaps.
Regards,
--
Alex
--
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
