LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: James Liao <jamesjj.liao@mediatek.com>,
	Mike Turquette <mturquette@linaro.org>,
	YH Chen <yh.chen@mediatek.com>,
	linux-kernel@vger.kernel.org,
	Henry Chen <henryc.chen@mediatek.com>,
	linux-mediatek@lists.infradead.org, kernel@pengutronix.de,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Yingjoe Chen <Yingjoe.Chen@mediatek.com>,
	Eddie Huang <eddie.huang@mediatek.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/5] clk: mediatek: Add basic clocks for Mediatek MT8135.
Date: Tue, 17 Mar 2015 14:54:31 +0100	[thread overview]
Message-ID: <20150317135431.GH10068@pengutronix.de> (raw)
In-Reply-To: <20150317094738.GF4927@pengutronix.de>

Hello Sascha,

On Tue, Mar 17, 2015 at 10:47:38AM +0100, Sascha Hauer wrote:
> On Mon, Mar 16, 2015 at 08:03:39AM +0100, Uwe Kleine-König wrote:
> > On Mon, Mar 16, 2015 at 06:55:09AM +0100, Sascha Hauer wrote:
> > > +static const char *axi_parents[] __initconst = {
> > > +	"clk26m",
> > > +	"syspll_d3",
> > > +	"syspll_d4",
> > > +	"syspll_d6",
> > > +	"univpll_d5",
> > > +	"univpll2_d2",
> > > +	"syspll_d3p5"
> > > +};
> > __initconst should only be used for const data, but this array isn't
> > const. Only the strings the pointer in this array point to are constant.
> 
> That may be true, but the way it's currently done compiles through
> without warnings with CONFIG_DEBUG_SECTION_MISMATCH enabled.
CONFIG_DEBUG_SECTION_MISMATCH doesn't catch this type of error.

> 
> > 
> > So you have to use either
> > 
> > 	static const char *axi_parents[] __initdata = {
> 
> This results in:
> 
> drivers/clk/mediatek/clk-mt8173.c:515:20: error: i2s3_b_ck_parents causes a section type conflict with infra_clks
>  static const char *i2s3_b_ck_parents[] __initdata = {
The compiler catches if a single section contains both modifiable and
constant data. The linker doesn't seem to check anything here. So you
have to do it consistently per compilation unit. (I'm not aware of any
problems if you do it consistently wrong in a compilation unit, but
better do it consistently right :-)
I'm sure getting it right isn't that hard, so IMHO this should be fixed
before your patch is applied.

> which can be avoided with static const char * const axi_parents[] __initdata
This should be __initconst (to do it consistently right).

> > or
> > 
> > 	static const char * const axi_parents[] __initconst = {
> 
> Which results in:
> 
> drivers/clk/mediatek/clk-mt8173.c:568:2: warning: initialization discards 'const' qualifier from pointer target type
>   MUX_GATE(TOP_HDCP_24M_SEL, "hdcp_24m_sel", hdcp_24m_parents, 0x00d0, 16, 2, 23),
> 
> With the following patch this can also be resolved. Until this is merged
> (or I have the OK from Mike to add this in front of this series) I
> prefer to keep it like it is at the moment.
I remember trying something like that some time ago, but without
success. I didn't modify clk-composite.c but still I would be surprised
if it were that easy.

(The result were commits like 4a043d79dc2d and 145047de9940.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

  reply	other threads:[~2015-03-17 13:54 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-16  5:55 [PATCH v7]: clk: Add common clock support for Mediatek MT8135 and MT8173 Sascha Hauer
2015-03-16  5:55 ` [PATCH 1/5] clk: mediatek: Add initial common clock support for Mediatek SoCs Sascha Hauer
2015-03-16  5:55 ` [PATCH 2/5] clk: mediatek: Add reset controller support Sascha Hauer
2015-03-16  5:55 ` [PATCH 3/5] clk: mediatek: Add basic clocks for Mediatek MT8135 Sascha Hauer
2015-03-16  7:03   ` Uwe Kleine-König
2015-03-17  9:47     ` Sascha Hauer
2015-03-17 13:54       ` Uwe Kleine-König [this message]
2015-03-16  5:55 ` [PATCH 4/5] clk: mediatek: Add basic clocks for Mediatek MT8173 Sascha Hauer
2015-03-16  5:55 ` [PATCH 5/5] dt-bindings: ARM: Mediatek: Document devicetree bindings for clock/reset controllers Sascha Hauer
  -- strict thread matches above, loose matches on Subject: below --
2015-02-22 11:49 [PATCH v6]: clk: Add common clock support for Mediatek MT8135 and MT8173 Sascha Hauer
2015-02-22 11:49 ` [PATCH 3/5] clk: mediatek: Add basic clocks for Mediatek MT8135 Sascha Hauer
2015-03-13  7:44   ` Henry Chen
2015-03-16  5:33     ` Sascha Hauer
2015-03-15 16:27   ` Daniel Kurtz
2015-03-16  5:33     ` Sascha Hauer

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=20150317135431.GH10068@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=Yingjoe.Chen@mediatek.com \
    --cc=eddie.huang@mediatek.com \
    --cc=henryc.chen@mediatek.com \
    --cc=jamesjj.liao@mediatek.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mturquette@linaro.org \
    --cc=s.hauer@pengutronix.de \
    --cc=yh.chen@mediatek.com \
    --subject='Re: [PATCH 3/5] clk: mediatek: Add basic clocks for Mediatek MT8135.' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).