From mboxrd@z Thu Jan 1 00:00:00 1970 From: "G, Manjunath Kondaiah" Subject: Re: [PATCH 3/4] dt: omap3: add generic board file for dt support Date: Mon, 18 Jul 2011 15:45:57 +0530 Message-ID: References: <1310592975-25773-1-git-send-email-manjugk@ti.com> <1310592975-25773-4-git-send-email-manjugk@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: linux-omap-owner@vger.kernel.org To: Grant Likely Cc: Kevin Hilman , Tony Lindgren , devicetree-discuss@lists.ozlabs.org, linux-omap@vger.kernel.org, ben-linux@fluff.org, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org Hi Grant, On 17 July 2011 10:43, Grant Likely wrote: > Hi Manjunath, > > Comments below. =A0I 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 wrote: >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely wrote: >>>> > +static void __init omap3_init(void) >>>> > +{ >>>> > + =A0 =A0 =A0 struct device_node *node; >>>> > + [...] >> +static struct omap_device *of_omap_i2c_device_create(struct device_= node *node, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0const char *bus_id, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0void *platform_data, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0struct device *parent) >> +{ >> + =A0 =A0 =A0 struct platform_device *pdev; >> + =A0 =A0 =A0 struct i2c_board_info *i2c_board_info; >> + =A0 =A0 =A0 u8 id; >> + >> + =A0 =A0 =A0 printk("Creating omap i2c device %s\n", node->full_nam= e); >> + >> + =A0 =A0 =A0 if (!of_device_is_available(node)) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL; >> + >> + =A0 =A0 =A0 id =3D simple_strtol(bus_id, NULL, 0); >> + =A0 =A0 =A0 pdev =3D kzalloc(sizeof(*pdev), GFP_KERNEL); >> + =A0 =A0 =A0 pdev->dev.of_node =3D of_node_get(node); >> + =A0 =A0 =A0 if (!pdev->dev.of_node) { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 speed =3D 100; >> + =A0 =A0 =A0 } else { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 prop; >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!of_property_read_u32(pdev->dev.of= _node, "clock-frequency", >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= &prop)) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 speed =3D prop/100; >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 speed =3D 100; >> + =A0 =A0 =A0 } >> + =A0 =A0 =A0 printk("%s : speed->%d\n",__func__, speed); >> + >> + =A0 =A0 =A0 for_each_child_of_node(bus, child) { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 prop; >> + >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(" =A0 create child: %s\n", chil= d->full_name); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 i2c_board_info =3D kzalloc(sizeof(*i2c= _board_info), GFP_KERNEL); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!of_property_read_u32(pdev->dev.of= _node, "reg", >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &prop)) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 i2c_board_info->addr =3D prop; >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!of_property_read_u32(pdev->dev.of= _node, "interrupts", >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &prop)) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 i2c_board_info->irq =3D prop; >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 i2c_board_info->platform_data =3D plat= form_data; >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strncpy(i2c_board_info->type, child->n= ame, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 sizeof(i2c_board_info->type)); >> + =A0 =A0 =A0 } >> + =A0 =A0 =A0 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. =A0= 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 dr= iver with this approach. BTW, I have observed that, if we create i2c device node in omapx-soc.dt= si file and the define i2c bus clock-frequency in beagle.dts, the clock-fr= equency is not available in driver probe. Is this expected behavior? -M -- To unsubscribe from this list: send the line "unsubscribe linux-omap" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: manjugk@ti.com (G, Manjunath Kondaiah) Date: Mon, 18 Jul 2011 15:45:57 +0530 Subject: [PATCH 3/4] dt: omap3: add generic board file for dt support In-Reply-To: References: <1310592975-25773-1-git-send-email-manjugk@ti.com> <1310592975-25773-4-git-send-email-manjugk@ti.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Grant, On 17 July 2011 10:43, Grant Likely 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 wrote: >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely 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. 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? -M