[PATCH 3/4] dt: omap3: add generic board file for dt support

[grant.likely@secretlab.ca (Grant Likely)]

On Mon, Jul 18, 2011 at 03:45:57PM +0530, G, Manjunath Kondaiah wrote:
> Hi Grant,
> 
> On 17 July 2011 10:43, Grant Likely <grant.likely@secretlab.ca> wrote:
> > Hi Manjunath,
> >
> > Comments below. ?I left in a lot of context for the new folks that
> > I've cc'd (Tony and Kevin).
> >
> > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >>>> > +static void __init omap3_init(void)
> >>>> > +{
> >>>> > + ? ? ? struct device_node *node;
> >>>> > +
> [...]
> >> +static struct omap_device *of_omap_i2c_device_create(struct device_node *node,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *bus_id,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *platform_data,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device *parent)
> >> +{
> >> + ? ? ? struct platform_device *pdev;
> >> + ? ? ? struct i2c_board_info *i2c_board_info;
> >> + ? ? ? u8 id;
> >> +
> >> + ? ? ? printk("Creating omap i2c device %s\n", node->full_name);
> >> +
> >> + ? ? ? if (!of_device_is_available(node))
> >> + ? ? ? ? ? ? ? return NULL;
> >> +
> >> + ? ? ? id = simple_strtol(bus_id, NULL, 0);
> >> + ? ? ? pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
> >> + ? ? ? pdev->dev.of_node = of_node_get(node);
> >> + ? ? ? if (!pdev->dev.of_node) {
> >> + ? ? ? ? ? ? ? speed = 100;
> >> + ? ? ? } else {
> >> + ? ? ? ? ? ? ? u32 prop;
> >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop))
> >> + ? ? ? ? ? ? ? ? ? ? ? speed = prop/100;
> >> + ? ? ? ? ? ? ? else
> >> + ? ? ? ? ? ? ? ? ? ? ? speed = 100;
> >> + ? ? ? }
> >> + ? ? ? printk("%s : speed->%d\n",__func__, speed);
> >> +
> >> + ? ? ? for_each_child_of_node(bus, child) {
> >> + ? ? ? ? ? ? ? u32 prop;
> >> +
> >> + ? ? ? ? ? ? ? printk(" ? create child: %s\n", child->full_name);
> >> + ? ? ? ? ? ? ? i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL);
> >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "reg",
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop))
> >> + ? ? ? ? ? ? ? i2c_board_info->addr = prop;
> >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "interrupts",
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop))
> >> + ? ? ? ? ? ? ? i2c_board_info->irq = prop;
> >> + ? ? ? ? ? ? ? i2c_board_info->platform_data = platform_data;
> >> + ? ? ? ? ? ? ? strncpy(i2c_board_info->type, child->name,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(i2c_board_info->type));
> >> + ? ? ? }
> >> + ? ? ? omap_register_i2c_bus(id, speed, i2c_board_info, 1);
> >
> > While this does in a sense work, and creates an omap_device for the
> > i2c bus that does get probed, it ends up encoding an awful lot of
> > device specific details into the generic devicetree support code. ?The
> > i2c bus driver itself must be responsible for decoding the speed and
> > child nodes, and in fact it can easily be modified to do so (I've
> 
> Decoding speed in i2c driver seems to be fine. But the i2c child nodes are
> board specific. We might bring board specific handling code into i2c driver
> with this approach.

It shouldn't.  They're just i2c devices, and the child nodes will
always follow the i2c device binding.

> BTW, I have observed that, if we create i2c device node in omapx-soc.dtsi
> file and the define i2c bus clock-frequency in beagle.dts, the clock-frequency
> is not available in driver probe. Is this expected behavior?

No, it sounds like something isn't getting set up correctly.  Send me
your current beagle.dts and omap3-soc.dtsi files.

g.