linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: jgunthorpe@obsidianresearch.com (Jason Gunthorpe)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 1/5] drivers: memory: Introduce Marvell EBU Device Bus driver
Date: Mon, 8 Apr 2013 11:19:40 -0600	[thread overview]
Message-ID: <20130408171940.GA8815@obsidianresearch.com> (raw)
In-Reply-To: <1365419194-20871-2-git-send-email-ezequiel.garcia@free-electrons.com>

Looks OK to me, though with mbus dt bindings we'd see some more
changes.

Just to note on them for future:

> +	child = of_get_next_child(node, NULL);
> +	if (!child) {
> +		dev_err(dev, "%s has no childs\n", node->full_name);
> +		return -EINVAL;
> +	}

This isn't really necessary anymore, the ranges of the node itself
should be parsed to get the window start/end. The driver should never
need to look at the children directly..

> +	/* Get the CS to choose the window string */
> +	err = of_property_read_u32(node, "ranges", &cs);
> +	if (err < 0)
> +		return err;

.. and this is why you've kept the 2 cells address - the top cell is
still encoding the target id. This would go away eventually too..

Maybe it would be better in the interm to compute the CS offset from
the control register offset?

(reg.start % 0x400)/8 should do the trick?

> +	devbus->child = of_platform_device_create(child, NULL, &pdev->dev);
> +	if (!devbus->child) {
> +		dev_warn(dev, "cannot create child device %s\n", child->name);
> +		/* Remove the allocated window */
> +		mvebu_mbus_del_window(devbus->child_mem.start,
> +				      resource_size(&devbus->child_mem));
> +	}

This can probably just be of_platform_populate or similar to do all
children? For instance, I often use many DT nodes to represent a FPGA,
since the my FPGA's tend to have many functionally orthogonal units
inside.

Regards,
Jason

  reply	other threads:[~2013-04-08 17:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-08 11:06 [PATCH v4 0/5] Device Bus support for Marvell EBU SoC Ezequiel Garcia
2013-04-08 11:06 ` [PATCH v4 1/5] drivers: memory: Introduce Marvell EBU Device Bus driver Ezequiel Garcia
2013-04-08 17:19   ` Jason Gunthorpe [this message]
2013-04-08 19:15     ` Arnd Bergmann
2013-04-09  0:42     ` Ezequiel Garcia
2013-04-09  9:40       ` Arnd Bergmann
2013-04-09 10:34         ` Ezequiel Garcia
2013-04-09 10:44           ` Arnd Bergmann
2013-04-09 11:00             ` Ezequiel Garcia
2013-04-09 16:00             ` Jason Gunthorpe
2013-04-09 16:03       ` Jason Gunthorpe
2013-04-09 20:28         ` Ezequiel Garcia
2013-04-08 11:06 ` [PATCH v4 2/5] ARM: mvebu: Add Device Bus support for Armada 370/XP SoC Ezequiel Garcia
2013-04-08 11:06 ` [PATCH v4 3/5] ARM: mvebu: Add support for NOR flash device on Armada XP-GP board Ezequiel Garcia
2013-04-08 11:06 ` [PATCH v4 4/5] ARM: mvebu: Add support for NOR flash device on Openblocks AX3 board Ezequiel Garcia
2013-04-08 11:06 ` [PATCH v4 5/5] ARM: mvebu: Add Device Bus and CFI flash memory support to defconfig Ezequiel Garcia
2013-04-08 14:17 ` [PATCH v4 0/5] Device Bus support for Marvell EBU SoC Arnd Bergmann
2013-04-08 15:29   ` Ezequiel Garcia
2013-04-08 18:30     ` Jason Cooper
2013-04-08 19:10       ` Jason Cooper

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=20130408171940.GA8815@obsidianresearch.com \
    --to=jgunthorpe@obsidianresearch.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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;
as well as URLs for NNTP newsgroup(s).