From: khilman@baylibre•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC] ARM: memory: da8xx-ddrctl: new driver
Date: Mon, 24 Oct 2016 10:35:30 -0700 [thread overview]
Message-ID: <7hd1ipzm3h.fsf@baylibre.com> (raw)
In-Reply-To: <20161024170035.GO15620@leverpostej> (Mark Rutland's message of "Mon, 24 Oct 2016 18:00:36 +0100")
Hi Mark,
Mark Rutland <mark.rutland@arm•com> writes:
> On Mon, Oct 24, 2016 at 06:46:36PM +0200, Bartosz Golaszewski wrote:
>> Create a new driver for the da8xx DDR2/mDDR controller and implement
>> support for writing to the Peripheral Bus Burst Priority Register.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre•com>
>> ---
>> .../memory-controllers/ti-da8xx-ddrctl.txt | 20 +++
>> drivers/memory/Kconfig | 8 +
>> drivers/memory/Makefile | 1 +
>> drivers/memory/da8xx-ddrctl.c | 187 +++++++++++++++++++++
>> 4 files changed, 216 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> create mode 100644 drivers/memory/da8xx-ddrctl.c
>>
>> diff --git
>> a/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> new file mode 100644
>> index 0000000..f0eda59
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> @@ -0,0 +1,20 @@
>> +* Device tree bindings for Texas Instruments da8xx DDR2/mDDR memory controller
>> +
>> +The DDR2/mDDR memory controller present on Texas Instruments da8xx SoCs memory
>> +maps a set of registers which allow to tweak the controller's behavior.
>
> This is a description of the *driver*. The device itself doesn't map
> some registers, it features them. Please descrive the *device*.
>
>> +
>> +Documentation:
>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
>> +
>> +Required properties:
>> +
>> +- compatible: "ti,da850-ddrctl" - for da850 SoC based boards
>
> Perhaps:
>
> "ti,da850-ddr-controller"
>
>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> +{
>> + const struct da8xx_ddrctl_config_knob *knob;
>> + const struct da8xx_ddrctl_setting *setting;
>> + u32 regprop[2], base, memsize, reg;
>> + struct device_node *node, *parent;
>> + void __iomem *ddrctl;
>> + const char *board;
>> + struct device *dev;
>> + int ret;
>> +
>> + dev = &pdev->dev;
>> + node = dev->of_node;
>> +
>> + /* Find the board name. */
>> + for (parent = node;
>> + !of_node_is_root(parent);
>> + parent = of_get_parent(parent));
>> +
>> + ret = of_property_read_string(parent, "compatible", &board);
>> + if (ret) {
>> + dev_err(dev, "unable to read the soc model\n");
>> + return ret;
>> + }
>
> I can see that you want to expose sysfs knobs for this, but is it really
> necessary to match boards like this? It's very fragile, and commits us
> to maintaining a database of board data (i.e. a board file).
>
> I am very much not keen on that.
The original proposal[1] was to create DT properties reflecting the
various knobs in the DDR Controller, but that was frowned upon since
that was more HW configuration than hardware description.
That resulted in this approach which keeps the HW configuration values
in the driver, and selectable based on DT compatible.
IMO, neither aproach is pretty. From a DT maintainer perspective, can
you comment on your preference?
Thanks,
Kevin
[1] https://marc.info/?l=linux-arm-kernel&m=147672200715076&w=2
next prev parent reply other threads:[~2016-10-24 17:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-24 16:46 [RFC] da850: DDR2/mDDR memory controller driver Bartosz Golaszewski
2016-10-24 16:46 ` [RFC] ARM: memory: da8xx-ddrctl: new driver Bartosz Golaszewski
2016-10-24 17:00 ` Mark Rutland
2016-10-24 17:35 ` Kevin Hilman [this message]
2016-10-24 17:42 ` Mark Rutland
2016-10-24 18:41 ` Kevin Hilman
2016-10-24 18:52 ` Kevin Hilman
2016-10-25 10:17 ` Bartosz Golaszewski
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=7hd1ipzm3h.fsf@baylibre.com \
--to=khilman@baylibre$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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