public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lubomir Popov <lpopov@mm-sol.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] OMAP5: Add support for the SOM5_EVB board (OMAP5430-based)
Date: Wed, 15 May 2013 13:46:36 +0300	[thread overview]
Message-ID: <5193678C.9@mm-sol.com> (raw)
In-Reply-To: <51934F8D.4010207@ti.com>

Hi Sricharan,

On 15/05/13 12:04, Sricharan R wrote:
> Hi,
> 
> On Wednesday 15 May 2013 01:25 PM, Lubomir Popov wrote:
>> Hi Sricharan,
>>
>> On 15/05/13 08:11, Sricharan R wrote:
>>> Hi,
>>> On Tuesday 14 May 2013 10:11 PM, Tom Rini wrote:
>>>> On Tue, May 14, 2013 at 07:09:33PM +0300, Lubomir Popov wrote:
>>>>> Hi Tom,
>>>>>
>>>>> On 14/05/13 17:52, Tom Rini wrote:
>>>>>> On Tue, May 14, 2013 at 01:24:41PM +0300, Lubomir Popov wrote:
>>>>>>> Hi Tom,
>>>>>>>
>>>>>>> I'm currently busy with other work; on the other hand, careful
>>>>>>> rebasing shall require some time, especially the Palmas stuff.
>>>>>>> What would be the deadline for a V2 submission?
>>>>>>>
>>>>>>> Meanwhile could you please have a look at the (already old)
>>>>>>> http://patchwork.ozlabs.org/patch/232743/? A simple patch,
>>>>>>> shall be needed if we enable USB (for the uEVM along with
>>>>>>> our board). In general, what are your plans regarding USB
>>>>>>> (.../patch/232742/)?
>>>>>> Thanks for the reminder, I'll grab 232743 soon.  232742 looks OK, but do
>>>>>> you have a patch around for uEVM still?
>>>>> Not yet (didn't have the opportunity to test, although some uEVMs should
>>>>> be around at MMS). As you know, a patch shall be needed in the uEVM board
>>>>> file along with the common USB stuff.
>>>> Yeah, I can test it as well if you write it up, and may find the time if
>>>> you point me in the right direction.
>>>>
>>>>>>> And again on I2C (.../patch/233823/): what is you final
>>>>>>> opinion? I'm confident that this patch is a major improvement
>>>>>>> for OMAP4/5 at least.
>>>>>> I'm inclined to go with it, just need to mentally unswap the i2c notes
>>>>>> in my brain and think it over one more time.
>>>>> Just applied 233823 to current u-boot-ti master. Works fine.
>>>> OK, thanks.
>>>>
>>>>>> [snip]
>>>>>>>>>>> +	 * TODO: Replace this ugly hardcoding with proper defines +
>>>>>>>>>>> */ +	writel(0x0100, 0x4ae0a310);
>>>>>>>>>> Again, do please.
>>>>>>>>> This should be (*scrm)->auxclk0. The problem is that the
>>>>>>>>> omap5_scrm_regs struct (holding the auxclk0 member) has to be
>>>>>>>>> defined somewhere in the common OMAP5 headers. Sricharan? Or should
>>>>>>>>> I hack around?
>>>>>>>> Add it to the most likely struct in the headers.
>>>>>>> The entire struct (I call it omap5_scrm_regs in theory, similar to the
>>>>>>> corresponding omap4_scrm_regs for OMAP4) is not defined anywhere. Of
>>>>>>> course I could define only the member that I need, but I guess it is
>>>>>>> a (responsible) TI job to define hardware descriptors. Or I'm wrong?
>>>>>>> Please advise. If I have time, I could do it myself - it's some 27
>>>>>>> registers, almost identical to the OMAP4, and should go into 
>>>>>>> arch/arm/include/asm/arch-omap5/clocks.h.
>>>>>> Whomever uses / needs it should do it.  I gave the TRM a quick read and
>>>>>> I don't see any conflicts per-se just some reserved areas being named
>>>>>> and vice versa.  So rename it to omap_scrm_regs and move to
>>>>>> <asm/omap_common.h>.  Thanks!
>>>>> I would argue that this is not very appropriate. Those regs that are
>>>>> reserved on the OMAP5 are related to altclkscr and auxclk5 on the OMAP4;
>>>>> on the other hand the OMAP5 has some modem clock regs that are reserved
>>>>> on OMAP4. We shall probably have ugly #ifdefs again. And what about OMAP3
>>>>> and below?
>>>> We don't need to use ifdefs since there's no conflicts, things are
>>>> either reserved in one case and used in the other.  And we can make sure
>>>> we don't try and use the omap5 bits on omap4 and vice versa.  I don't
>>>> see scrm in the first omap3 TRM I pulled up, so we don't need to worry
>>>> there.
>>>>
>>>>> Currently the scrm struct is defined for OMAP4 in the asm/arch-omap4/clocks.h
>>>>> file and I have already done the same for OMAP5 by analogy. I must admit
>>>>> however that this approach does not correspond to the latest way by which
>>>>> groups of OMAP hardware regs are defined, prcm in particular - a struct in
>>>>> omap_common.h, holding only the required regs, no padding and such garbage,
>>>>> and an init with the physical addresses in a .c file for the particular SoC
>>>>> (prcm-regs.c). But still the Panda board, for example, uses the old way for
>>>>> scrm. Therefore I did it the same for OMAP5, which was easier (I'm old and
>>>>> lazy ;) ).
>>>> Yes, I'm OK starting off with moving things into omap_common.h as-is and
>>>> then updating them a bit later ala pcrm-regs.c.
>>>>
>>>>
>>>  I am sorry for the very late response on this.
>>>  Now then, why not add this register in to omap5_es2_prcm
>>>  ??. That is how the  TRM sees it as well.. Of course, this is cleanup
>>>  stuff for OMAP4  panda as you pointed out..
>> Yes, you are right in respect to fluent software integration and consistency
>> with current implementation. My only concern is that from architectural point
>> of view the SCRM, although related to the PRCM, is a separate module (described
>> correspondingly as such in the TRM). If we go this way, the SCRM regs shall
>> have to be referenced via the prcm pointer: (*prcm)->xxxxx, and this might be
>> confusing.
>>
>> I'm OK to do it as above, that is, add the SCRM regs (for both OMAP4 and OMAP5)
>> to the prcm_regs declaration in omap_common.h, and the required init to the
>> appropriate omap5_esx_prcm in prcm-regs.c, but would suggest that for improved
>> clarity the SCRM register names, as they now exist in .../arch-omap4/clocks.h,
>> start with a scrm_ prefix.
>>
>> Alternatively, a new scrm_regs struct could be declared in omap_common.h, along
>> with the appropriate pointers, and the corresponding definitions/inits added as
>> new structs in prcm-regs.c for every SoC. But then the file name prcm-regs shall
>> be a bit confusing.
>>
>> What do you say?
>   At this point , i think just adding only the registers that you need in to the
>  prcm structure should be ok.
OK, I just tend to add all registers, it shall be easier for later rework.
> 
> Regards,
>  Sricharan
> 

BR,
Lubo

  reply	other threads:[~2013-05-15 10:46 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-01 14:06 [U-Boot] [PATCH] OMAP5: Add support for the SOM5_EVB board (OMAP5430-based) Lubomir Popov
2013-04-25 19:01 ` Tom Rini
2013-04-26 15:59   ` Lubomir Popov
2013-05-13 19:37     ` Tom Rini
2013-05-14 10:24       ` Lubomir Popov
2013-05-14 14:52         ` Tom Rini
2013-05-14 16:09           ` Lubomir Popov
2013-05-14 16:41             ` Tom Rini
2013-05-14 19:42               ` Lubomir Popov
2013-05-14 20:36                 ` Tom Rini
2013-05-15  5:11               ` Sricharan R
2013-05-15  7:55                 ` Lubomir Popov
2013-05-15  9:04                   ` Sricharan R
2013-05-15 10:46                     ` Lubomir Popov [this message]
2013-05-15 11:25                       ` Sricharan R
2013-05-15 13:10                         ` Lubomir Popov
2013-05-15 13:43                           ` Tom Rini
2013-05-15 14:31                             ` Lubomir Popov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5193678C.9@mm-sol.com \
    --to=lpopov@mm-sol.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox