mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v2] clk: i.MX: Port Linux clock tree for i.MX51 and i.MX53
Date: Thu, 6 Sep 2018 09:48:34 +0200	[thread overview]
Message-ID: <20180906074834.kif6qcaqfcde26rt@pengutronix.de> (raw)
In-Reply-To: <20180830050207.13192-1-andrew.smirnov@gmail.com>

On Wed, Aug 29, 2018 at 10:02:07PM -0700, Andrey Smirnov wrote:
> Existing clock tree code for i.MX5 in Barebox predates DT and is not
> aware of it. This results in missing clocks on DT-based boards like
> RDU1 and Babbage. Port clock tree from Linux to resolve this
> problem. Old non-DT clock code is kept around for the sake of the
> boards that were never converted to use DT.

Overall I am not happy with this patch.

With this patch we now have two clock drivers for the i.MX5 - not only
in the source tree but also in the binaries.

Yesterday I tried fleshing out the differences between both drivers. I
renamed "clks" to "clk", adjusted whitespaces, changed register defines
to the pattern "#define MXC_CCM_xxx (ccm_base + 0x*)". What I got was
quite a bit closer to the kernel driver but still not there. It revealed
some bugs in the kernel driver though. There are several differences in
the register layout between the i.MX50 and the i.MX51/53 (See
IMX5_CLK_ESDHC_A_SEL for example, MXC_CCM_CSCMR1[20:21] on i.MX51/53 and
MXC_CCM_CSCMR1[21:22] on the i.MX50). These are correctly abstracted
in the current barebox driver but not in the Linux driver, see
mx5_clocks_mx51_mx53_init() which doesn't exist in the Linux driver.

So by switching to the Kernel clk driver we introduce a bunch of new
bugs into barebox which of course is unfortunate.

Finally your patch does not compile on some configs
(efika-mx-smartbook_defconfig for example) since COMMON_CLK_OF_PROVIDER
is not selected. That's rather simple to fix of course.

Which clocks are you missing? Maybe it would be better to add the
missing clocks to the barebox clock driver instead of adding a new one?

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  parent reply	other threads:[~2018-09-06  7:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-30  5:02 Andrey Smirnov
2018-09-04  6:37 ` Sascha Hauer
2018-09-06  7:48 ` Sascha Hauer [this message]
2018-09-06 15:59   ` Andrey Smirnov

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=20180906074834.kif6qcaqfcde26rt@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=andrew.smirnov@gmail.com \
    --cc=barebox@lists.infradead.org \
    /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