Re: [PATCH 1/2] ARM: dts: Enable RTC node in exynos4.dtsi file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wednesday 13 of November 2013 12:52:05 Bartlomiej Zolnierkiewicz wrote:

[+ DT maintainers]

> 
> Hi,
> 
> On Wednesday, November 13, 2013 11:27:03 AM Sylwester Nawrocki wrote:
> > On 13/11/13 09:08, Tomasz Figa wrote:
> > >> As was discussed earlier too, status field of DT node is not supposed
> > >> > to be used for
> > >> > keeping an IP enabled or disabled. That should be done via the kernel
> > >> > config. The DT status
> > >> > is mostly to indicate the hardware status of the IP on the SoC/board.
> > >> > If the node fully defines the hardware,
> > >> > then it should be kept enabled by default unless such enabling causes
> > >> > some issues with other IPs due to
> > >> > pin sharing conflicts, etc. In the above case the node completely
> > >> > defines the hardware and hence there is no
> > >> > reason to keep it disabled.
> > >
> > > That's correct. (Unless I'm missing some board specific dependency of RTC.
> > > If so, please correct me.)

Well, I was missing one indeed. RTC requires an external 32.768KHz clock,
which must be provided by an oscillator or any other clock source on the
board. However...

> > 
> > I don't really like this argument. Why not allow the firmware to decide
> > which devices are relevant and should be handled by the kernel ?
> > And since we are aiming at single kernel config, if I understand things
> > correctly, I can't see anything else than dts that could hold the machine
> > *configuration*.
> > 
> > So let's not make all stuff enabled by default, that's not something we
> > want on those mobile device SoCs. We should not be making fine system
> > tuning more difficult than necessary.
> > 
> > I'm with Kukjin on this matter and would prefer patches like the $subject
> > patch not be merged.
> 
> I generally agree with Sylwester and Kukjin that devices should not be
> enabled by default in dtsi files.  However in a particular case of RTC
> support there should be an exception from the generic rule and RTC
> should be enabled for all EXYNOS boards (we have RTC driver config
> option already enabled in our exynos_defconfig and we are also already
> enabling RTC device explicitly in EXYNOS5250 dtsi file).

I believe the rule is clear and we already went through enough discussion
about this. To clarify again:

Device Tree should not restrict possible use cases of particular hardware
that are allowed by particular integration of such hardware on particular
board. This means that if there are no technical reasons to mark such
hardware as disabled on particular board, this should not be done. Let
me show you this on several examples:

1. SoC X has an MMC controller, which needs N pins of the SoC to be
routed to an MMC slot. Our board X1 does _not_ have necessary traces on
its PCB. In this case the MMC controller must be marked as disabled.

2. SoC X has an MMC controller, which needs N pins of the SoC to be
routed to an MMC slot. Our board X2 _does_ have necessary traces on its
PCB, so when user inserts an MMC card into the slot he expects that the
system detects it. In this case the MMC controller must _not_ be marked
as disabled.

3. SoC X has a built-in 2D graphics accelerator. It does not have any
output pads nor any requirements with respect to the board - it's purely
a mem-to-mem device. A user might want to run a rootfs that uses it to
accelerate his applications, so this device must _not_ be marked as
disabled.

4. SoC X has a image processing block, consisting of two functions:
 - a camera interface, that requires SoC pads to be connected to a camera
   sensor,
 - general-purpose image scaler and format converter, that can operate
   either on input of camera interface or on images stored in memory.
This case is more interesting. Even if board does not have a physical
camera interface, the binding should be designed in a way that allows
enabling only the memory-to-memory scaler. Then such IP must be marked
okay regardless of board support, because the camera interface will be
used only if relevant board-specific properties are provided.

To sum up, I would not interpret the "disabled" value of "status" property
as the opposite of "enabled", but rather as "disabled" in "disabled
person".

Anyway, I'd like to get a confirmation (or denial) from other Device Tree
maintainers and if what I've written above is correct, maybe we should
put this somewhere in kernel documentation.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux