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

[Grant Likely <grant.likely@secretlab.ca>]

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.

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