Linux Samsung SOC development
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: Tomasz Figa <t.figa@samsung.com>
Cc: Sachin Kamat <sachin.kamat@linaro.org>,
	Leela Krishna Amudala <l.krishna@samsung.com>,
	linux-samsung-soc@vger.kernel.org, kgene.kim@samsung.com,
	dianders@chromium.org, jg1.han@samsung.com,
	grant.likely@secretlab.ca, arnd@arndb.de, olof@lixom.net,
	s.nawrocki@samsung.com
Subject: Re: [PATCH 2/3] ARM: exynos5420: dt: add clock entries to watchdog node
Date: Thu, 25 Jul 2013 11:18:51 -0700	[thread overview]
Message-ID: <51F16C0B.7070107@wwwdotorg.org> (raw)
In-Reply-To: <5919446.9SmJVKqkD3@amdc1227>

On 07/24/2013 07:14 AM, Tomasz Figa wrote:
> On Wednesday 24 of July 2013 16:51:06 Sachin Kamat wrote:
...
>> This is contrary to the fact that we disable everything by default in
>> the top level dt files and only enable them as required in the board
>> dts files.
> 
> No, we don't disable everything. We disable things that require board 
> specific setup or can't work without other support from board side. If there 
> is some hardware disabled in SoC level dtsi that can work without any 
> support from board side, then it is a BUG() and must be fixed.
> 
>>> To illustrate the problem please consider that in the end, a dtb file
>>> will be fused into board ROM (or at most flash memory) and passed to
>>> the kernel by the bootloader. If you disable some hardware on DT level
>>> even if it can be physically used on the board, there will be no way
>>> to reenable it, if some user wanted to use it, because that would
>>> require editing the fused dtb.
>>
>> I believe some h/w will be disabled in dt only if it should not be
>> used for whatever reason. If there is no reason then ofcourse they
>> would be enabled IMHO.
> 
> Yes. This is what I meant. However the reason must be valid - e.g. 
> "hardware does not allow such configuration", not like "some very important 
> manager decided that this board should not use this".
> 
>> Whatever be the case the choice of enabling or
>> disabling should be done at the leaf node (at board level). No?
> 
> It depends. For components that don't require any support from board side 
> it can be globally enabled on SoC level. If a SoC component requires 
> support from board side (like regulators, GPIOs, etc.) then it should be 
> disabled on SoC level and enabled on board level only if all the 
> dependencies are provided.

I tend to agree with Tomasz.

One note though: This is talking about the *.dts files, which may be
different from the DTB that gets passed to the kernel.

In other words, I don't think that the SoC or board .dtsi file (at least
public versions that are hosted outside company-internal/downstream
branches) have any business disabling HW based on policy or use-cases,
rather than actual HW issues such as requiring other HW to support it
that isn't present or doesn't work.

However, I don't think anyone can influence what e.g.a bootloader does
to the DTB before passing it to the kernel; it could add
status="disabled" to some nodes based on policy, and nobody in the Linux
kernel has any absolute right to influence it, although there's sure a
right to complain about it and point out why it's a bad idea.

Equally, if somebody is creating a "BSP" (ick!) for a specific product's
production flash image, rather than contributing to upstream SW support
for that HW board, we probably don't have too much say in what they do.

  reply	other threads:[~2013-07-25 18:18 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-24  9:38 [PATCH 0/3] parse watchdog node to read PMU registers addresses Leela Krishna Amudala
2013-07-24  9:38 ` [PATCH 1/3] watchdog: s3c2410_wdt: parse watchdog dt node to read PMU registers adresses Leela Krishna Amudala
2013-07-24 12:39   ` Kukjin Kim
2013-07-25 10:27   ` Tomasz Figa
2013-07-26  0:32     ` Doug Anderson
2013-08-10 21:32       ` Olof Johansson
2013-07-24  9:38 ` [PATCH 2/3] ARM: exynos5420: dt: add clock entries to watchdog node Leela Krishna Amudala
2013-07-24  9:48   ` Sachin Kamat
2013-07-24  9:54     ` Tomasz Figa
2013-07-24 10:01       ` Sachin Kamat
2013-07-24 10:44         ` Leela Krishna Amudala
2013-07-24 11:12         ` Tomasz Figa
2013-07-24 11:21           ` Sachin Kamat
2013-07-24 11:56             ` Kukjin Kim
2013-07-24 14:09               ` Tomasz Figa
2013-07-24 14:52                 ` Sylwester Nawrocki
2013-07-24 15:23                   ` Tomasz Figa
2013-07-24 14:14             ` Tomasz Figa
2013-07-25 18:18               ` Stephen Warren [this message]
2013-07-24  9:38 ` [PATCH 3/3] ARM: dts: exynos: add PMU registers addresses and mask bit " Leela Krishna Amudala
2013-07-24  9:46   ` Sachin Kamat
2013-07-24 10:54     ` Leela Krishna Amudala
2013-07-24 11:00       ` Sachin Kamat

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=51F16C0B.7070107@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=arnd@arndb.de \
    --cc=dianders@chromium.org \
    --cc=grant.likely@secretlab.ca \
    --cc=jg1.han@samsung.com \
    --cc=kgene.kim@samsung.com \
    --cc=l.krishna@samsung.com \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=olof@lixom.net \
    --cc=s.nawrocki@samsung.com \
    --cc=sachin.kamat@linaro.org \
    --cc=t.figa@samsung.com \
    /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