On 04/13/2016 07:49 AM, Vinod Koul wrote:
On Tue, Apr 12, 2016 at 05:23:32PM +0100, Jon Hunter wrote:
why should this be hard coded in kernel and not queried from something like
DT? This case seems to be hardware property
Originally, I did have this in DT, however, the Tegra maintainers prefer
this and this is consistent with the other Tegra DMA driver (see
driver/dma/tegra20-apb-dma.c) [0].
But this creates a problem when you have next generation of controller with
different channel count!
How do we solve then?
Same way we solve this for the tegra20-apb-dma driver by having
different chip data per SoC in the driver ...
1259 /* Tegra20 specific DMA controller information */
1260 static const struct tegra_dma_chip_data tegra20_dma_chip_data = {
1261 .nr_channels = 16,
1262 .channel_reg_size = 0x20,
1263 .max_dma_count = 1024UL * 64,
1264 .support_channel_pause = false,
1265 .support_separate_wcount_reg = false,
1266 };
1267
1268 /* Tegra30 specific DMA controller information */
1269 static const struct tegra_dma_chip_data tegra30_dma_chip_data = {
1270 .nr_channels = 32,
1271 .channel_reg_size = 0x20,
1272 .max_dma_count = 1024UL * 64,
1273 .support_channel_pause = false,
1274 .support_separate_wcount_reg = false,
1275 };
1276
1277 /* Tegra114 specific DMA controller information */
1278 static const struct tegra_dma_chip_data tegra114_dma_chip_data = {
1279 .nr_channels = 32,
1280 .channel_reg_size = 0x20,
1281 .max_dma_count = 1024UL * 64,
1282 .support_channel_pause = true,
1283 .support_separate_wcount_reg = false,
1284 };
1285
1286 /* Tegra148 specific DMA controller information */
1287 static const struct tegra_dma_chip_data tegra148_dma_chip_data = {
1288 .nr_channels = 32,
1289 .channel_reg_size = 0x40,
1290 .max_dma_count = 1024UL * 64,
1291 .support_channel_pause = true,
1292 .support_separate_wcount_reg = true,
1293 };
You may still say this should be in the DT, but the Tegra maintainers
prefer this data in the driver.
Okay I don't see a a rationale behind this not being in DT, Stephan?
The rationale for putting such data into DT is that when a new SoC comes
out, someone can simply modify the DT to change the parameter and the
existing driver will magically work on the new HW.
In practice, at least for HW blocks on Tegra, this doesn't turn out to
be true very often at all. We almost /always/ need some change to the
driver, because it needs to manipulate some additional clock, program
some new register/field, adapt to register/field layout changes, work
around bugs differently, etc. As such, expecting driver code changes is
the norm not the exception.
Given that, we have two choices about how to represent certain data in DT:
a) Put (hard-code) the data into the driver, and just use it. Perhaps
this data is in a table in order to support different values per SoC, as
in the code quoted above. This is very simple.
b) Put the data into DT, and write extra driver code to parse it, all
just to extract the exact same value we would have hard-coded into the
driver. This is useless overhead /unless/ it allows re-use of the driver
unchanged on new SoCs, which rarely happens.
As such, it's much simpler to choose option (a).
If an initial version of a driver hard-codes some value and by magic a
new version of the HW comes out for which the only difference is some
value like this, then:
a) It's likely that the value has increased rather than decreased on new
HW, so the existing hard-coded driver can work just as well as on old
HW, but simply not exposing all HW capabilities. This won't always be
true, but seems just as reasonable to expect as expecting the driver
won't need modifications for any other reason.
b) At the time we add driver support for the new chip, we can define a
new DT property that represents the previously hard-coded data. If the
property is present, its value is used. Otherwise the previous
hard-coded default can still be used. This allows us to retrospectively
realise that a DT property would have been a good idea, yet not bog the
code down with using the DT property until it is definitively known to
be useful.
Now, my points above all refer to Tegra HW blocks. Hopefully they don't
apply to IP vendors who sell their block to various SoC vendors. There,
it's much more obviously beneficial to allow DT-based configuration of
any IP block parameters, since different SoC vendors will almost
certainly pick different configuration values if it's possible to do so.
Note that this is more about different instances (parameterizations) of
the same version of an IP block though.
Equally,, my points apply to on-SoC HW blocks rather than anything board
specific. Board designs very obviously vary a lot, and it's obviously
beneficial to use DT to represent those differences (for consumption by
a generic OS; there's not so much benefit when talking about firmware
consuming DT).
--
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
