In the future, please do not top-post. Inline replies are preferred so context can be followed. I've moved your reply into context below with some more comments... "Karicheri, Muralidharan" <m-karicheri2@xxxxxx> writes: >>-----Original Message----- >>From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>Sent: Wednesday, January 06, 2010 11:04 AM >>To: Karicheri, Muralidharan >>Cc: linux-media@xxxxxxxxxxxxxxx; hverkuil@xxxxxxxxx; davinci-linux-open- >>source@xxxxxxxxxxxxxxxxxxxx >>Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc >>drivers to platform driver >> >>"Karicheri, Muralidharan" <m-karicheri2@xxxxxx> writes: >> >>>>> CLK(NULL, "rto", &rto_clk), >>>>> CLK(NULL, "usb", &usb_clk), >>>>> + CLK("dm355_ccdc", "master", &vpss_master_clk), >>>>> + CLK("dm355_ccdc", "slave", &vpss_slave_clk), >>>> >>>>I still don't understand why you have to add new entries here and >>>>can't simply rename the existing CLK nodes using vpss_*_clk. >>>> >>> >>> [MK] This will allow multiple drivers define their own clocks derived >>from >>> these. ccdc driver is not the only driver using these clocks. >> >>OK, but that still doesn't answer why you need multiple CLK() nodes. >> >>Who else is using the clocks? >> > > display, resizer drivers etc... OK, I'm not extremely familar with the whole video architecture here, but are all of these drivers expected to be doing clk_get() and clk_enable()? I thought the point of moving the clocks into the CCDC driver was so that the clock management was done in a single, shared space. Kevin >>> Your earlier suggestion was to use as follows :- >>> >>> - CLK(NULL, "vpss_master", &vpss_master_clk), >>> - CLK(NULL, "vpss_slave", &vpss_slave_clk), >>> + CLK("vpfe-capture", "master", &vpss_master_clk), >>> + CLK("vpfe-capture", "slave", &vpss_slave_clk), >>> >>> I am not sure if the following will work so that it can be used across >>> multiple drivers. >>> >>> + CLK(NULL, "master", &vpss_master_clk), >>> + CLK(NULL, "slave", &vpss_slave_clk), >>> >>> If yes, I can re-do this patch. Please confirm. >> >>No, this will not work. You need a dev_id field so that matching >>is done using the struct device. >> >>My original suggestion was when you had the VPFE driver doing the >>clk_get(). Now that it's in CCDC, maybe it should look like this. >> >>- CLK(NULL, "vpss_master", &vpss_master_clk), >>- CLK(NULL, "vpss_slave", &vpss_slave_clk), >>+ CLK("ccdc", "master", &vpss_master_clk), >>+ CLK("ccdc", "slave", &vpss_slave_clk), >> >>Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html