Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 2013-08-27 at 16:51 +0100, Rob Herring wrote: > >> * pl111 has no driver or binding in mainline, but appears in dts files. > >> Those dts files clcdclk and apb_pclk, in that order. We could fix > >> those before a driver starts using them. > > I think this was waiting for some generic display bindings? Pawel may know. Common Display Framework it was ;-) And yes, it was waiting for it, but it is getting bored with waiting so I'll cut off the CDF bits and post the rest this or next week. As to the APB clock, we've been there before... http://article.gmane.org/gmane.linux.ports.arm.kernel/188927 Cheers! Pawel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 27, 2013 at 9:08 AM, Lee Jones wrote: > On Tue, 27 Aug 2013, Mark Rutland wrote: > >> On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote: >> > On Fri, 23 Aug 2013, Mark Rutland wrote: >> > >> > > On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: >> > > > I had a short chat with Rob last night about this. I'm going to loop >> > > > him in to the conversation, as he wrote the binding. >> > > > >> > > > > > When most of the other clocks that we deal with are being >> > > > > > requested, >> > > > > > they rely on being index zero: >> > > > > > >> > > > > > drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, >> > > > > > NULL); >> > > > > >> > > > > Look at drivers/clk/clkdev.c, there's some fuzzy matching >> > > > > involved when you pass NULL as connection id. >> > > > >> > > > Yes, I've been looking at that. This is why it works currently. I >> > > > think I need to change all of the drivers to specify which clock they >> > > > want. At the moment that 'fuzzy matching' is what's saving us. If >> > > > anyone were to change our DTS file to match what the binding says, >> > > > then it would cease to work. I'm guessing this is the same for all >> > > > other DTS files too: >> > > >> > > I think if anything, the binding document(s) should be updated to >> > > describe that apb_pclk is referred to by name, and the names of the >> > > other clocks should be described in the specific device bindings. We can >> > > then modify the drivers which grab clock 0 to explicitly grab the first >> > > clock by name, and backwards compatibility should not be broken. >> > > >> > > I don't believe any other OS has implemented the common clock bindings, >> > > and we've never supported the binding as described. Let's correct the >> > > de-facto standard into a standard by decree. >> > >> > I think we need to respect, or at least take into consideration the >> > reason for the original 'de-facto' standard. Other OSes shouldn't be >> > forced to provide a named clock request in order to obtain >> > 'apb_pclk'. If the binding says it should be first, then perhaps we >> > should do just that. It's simply a matter of naming all subsequent >> > clocks related to AMBA devices. >> >> Ideally, yes. However, we have to be careful to not break compatibility. >> >> I took a look at existing primecell drivers and what they do. The >> situation isn't so bad (with the exception of the >> half-dt/half-platform-code mess): >> >> * Some don't deal with clocks at all (no clk* in the driver). pl320 is >> in the ecx-common dtsi with only apb_pclk but has no binding >> defined. Some have no clocks defined in the dt and are presumably few >> clocks by platform data or are non-functional. >> >> I'm not sure how these DTs are going to be supported if and when we >> remove the platform data they depend upon. If we're really going to do >> that, then they are clearly not supported as-is long term. >> >> * The pl022 driver grabs the first clock to figure out the rate of the >> spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in >> the binding. The ste-u300 dts has two clock-names, "apb_pclk" and >> "spi_clk" (in that order), but they refer to the same clock. >> >> Given the existing driver simply grabs the first clock and they're >> both the same, we could re-order the names and make the driver grab >> the second clock. That wouldn't break backwards compatibility with the >> sole dts file we have using the binding, though this assumes no-one >> else has a dt lying around with different clocks. >> >> * pl010 grabs the first clock given to it to figure out the uart rate >> (assuming it is UARTCLK), but it's only in integratorap.dts, without >> clocks, and is presumably fed by platform data. There is no binding >> document. >> >> pl011 grabs the first clock given to figure out the UART rate >> (assuming it is UARTCLK). The binding explicitly states it's only >> given apb_pclk, despite UARTCLK and PCLK being separate inputs to the >> IP block. >> >> These two bindings don't describe the hardware, and should be fixed. >> The only way I can think to make this work without breaknig backwards >> compatibility would be to try to grab the second clock and then fall >> back to the first if there isn't one. The other option is to break >> backwards compatibility, but I'm not sure that's much of an option. This was an oversight since highbank has a single clock. But yes, this should be 2 clocks. It should be fixed and in a compatible way please. >> * pl111 has no driver or binding in mainline, but appears in dts files. >> Those dts files clcdclk and apb_pclk, in that order. We could fix >> those before a driver starts using them. I think this was waiting for some generic display bindings? Pawel may know. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 27 Aug 2013, Mark Rutland wrote: > On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote: > > On Fri, 23 Aug 2013, Mark Rutland wrote: > > > > > On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: > > > > I had a short chat with Rob last night about this. I'm going to loop > > > > him in to the conversation, as he wrote the binding. > > > > > > > > > > When most of the other clocks that we deal with are being requested, > > > > > > they rely on being index zero: > > > > > > > > > > > > drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, > > > > > > NULL); > > > > > > > > > > Look at drivers/clk/clkdev.c, there's some fuzzy matching > > > > > involved when you pass NULL as connection id. > > > > > > > > Yes, I've been looking at that. This is why it works currently. I > > > > think I need to change all of the drivers to specify which clock they > > > > want. At the moment that 'fuzzy matching' is what's saving us. If > > > > anyone were to change our DTS file to match what the binding says, > > > > then it would cease to work. I'm guessing this is the same for all > > > > other DTS files too: > > > > > > I think if anything, the binding document(s) should be updated to > > > describe that apb_pclk is referred to by name, and the names of the > > > other clocks should be described in the specific device bindings. We can > > > then modify the drivers which grab clock 0 to explicitly grab the first > > > clock by name, and backwards compatibility should not be broken. > > > > > > I don't believe any other OS has implemented the common clock bindings, > > > and we've never supported the binding as described. Let's correct the > > > de-facto standard into a standard by decree. > > > > I think we need to respect, or at least take into consideration the > > reason for the original 'de-facto' standard. Other OSes shouldn't be > > forced to provide a named clock request in order to obtain > > 'apb_pclk'. If the binding says it should be first, then perhaps we > > should do just that. It's simply a matter of naming all subsequent > > clocks related to AMBA devices. > > Ideally, yes. However, we have to be careful to not break compatibility. > > I took a look at existing primecell drivers and what they do. The > situation isn't so bad (with the exception of the > half-dt/half-platform-code mess): > > * Some don't deal with clocks at all (no clk* in the driver). pl320 is > in the ecx-common dtsi with only apb_pclk but has no binding > defined. Some have no clocks defined in the dt and are presumably few > clocks by platform data or are non-functional. > > I'm not sure how these DTs are going to be supported if and when we > remove the platform data they depend upon. If we're really going to do > that, then they are clearly not supported as-is long term. > > * The pl022 driver grabs the first clock to figure out the rate of the > spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in > the binding. The ste-u300 dts has two clock-names, "apb_pclk" and > "spi_clk" (in that order), but they refer to the same clock. > > Given the existing driver simply grabs the first clock and they're > both the same, we could re-order the names and make the driver grab > the second clock. That wouldn't break backwards compatibility with the > sole dts file we have using the binding, though this assumes no-one > else has a dt lying around with different clocks. > > * pl010 grabs the first clock given to it to figure out the uart rate > (assuming it is UARTCLK), but it's only in integratorap.dts, without > clocks, and is presumably fed by platform data. There is no binding > document. > > pl011 grabs the first clock given to figure out the UART rate > (assuming it is UARTCLK). The binding explicitly states it's only > given apb_pclk, despite UARTCLK and PCLK being separate inputs to the > IP block. > > These two bindings don't describe the hardware, and should be fixed. > The only way I can think to make this work without breaknig backwards > compatibility would be to try to grab the second clock and then fall > back to the first if there isn't one. The other option is to break > backwards compatibility, but I'm not sure that's much of an option. > > * pl111 has no driver or binding in mainline, but appears in dts files. > Those dts files clcdclk and apb_pclk, in that order. We could fix > those before a driver starts using them. > > If you think those suggestions are OK, I can put together a series to > fix this. I think we need to hear from Rob before we proceed tbh, as he is the original author and should have a chance to voice his opinion. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote: > On Fri, 23 Aug 2013, Mark Rutland wrote: > > > On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: > > > I had a short chat with Rob last night about this. I'm going to loop > > > him in to the conversation, as he wrote the binding. > > > > > > > > When most of the other clocks that we deal with are being requested, > > > > > they rely on being index zero: > > > > > > > > > > drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, > > > > > NULL); > > > > > > > > Look at drivers/clk/clkdev.c, there's some fuzzy matching > > > > involved when you pass NULL as connection id. > > > > > > Yes, I've been looking at that. This is why it works currently. I > > > think I need to change all of the drivers to specify which clock they > > > want. At the moment that 'fuzzy matching' is what's saving us. If > > > anyone were to change our DTS file to match what the binding says, > > > then it would cease to work. I'm guessing this is the same for all > > > other DTS files too: > > > > I think if anything, the binding document(s) should be updated to > > describe that apb_pclk is referred to by name, and the names of the > > other clocks should be described in the specific device bindings. We can > > then modify the drivers which grab clock 0 to explicitly grab the first > > clock by name, and backwards compatibility should not be broken. > > > > I don't believe any other OS has implemented the common clock bindings, > > and we've never supported the binding as described. Let's correct the > > de-facto standard into a standard by decree. > > I think we need to respect, or at least take into consideration the > reason for the original 'de-facto' standard. Other OSes shouldn't be > forced to provide a named clock request in order to obtain > 'apb_pclk'. If the binding says it should be first, then perhaps we > should do just that. It's simply a matter of naming all subsequent > clocks related to AMBA devices. Ideally, yes. However, we have to be careful to not break compatibility. I took a look at existing primecell drivers and what they do. The situation isn't so bad (with the exception of the half-dt/half-platform-code mess): * Some don't deal with clocks at all (no clk* in the driver). pl320 is in the ecx-common dtsi with only apb_pclk but has no binding defined. Some have no clocks defined in the dt and are presumably few clocks by platform data or are non-functional. I'm not sure how these DTs are going to be supported if and when we remove the platform data they depend upon. If we're really going to do that, then they are clearly not supported as-is long term. * The pl022 driver grabs the first clock to figure out the rate of the spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in the binding. The ste-u300 dts has two clock-names, "apb_pclk" and "spi_clk" (in that order), but they refer to the same clock. Given the existing driver simply grabs the first clock and they're both the same, we could re-order the names and make the driver grab the second clock. That wouldn't break backwards compatibility with the sole dts file we have using the binding, though this assumes no-one else has a dt lying around with different clocks. * pl010 grabs the first clock given to it to figure out the uart rate (assuming it is UARTCLK), but it's only in integratorap.dts, without clocks, and is presumably fed by platform data. There is no binding document. pl011 grabs the first clock given to figure out the UART rate (assuming it is UARTCLK). The binding explicitly states it's only given apb_pclk, despite UARTCLK and PCLK being separate inputs to the IP block. These two bindings don't describe the hardware, and should be fixed. The only way I can think to make this work without breaknig backwards compatibility would be to try to grab the second clock and then fall back to the first if there isn't one. The other option is to break backwards compatibility, but I'm not sure that's much of an option. * pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them. If you think those suggestions are OK, I can put together a series to fix this. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Fri, 23 Aug 2013, Mark Rutland wrote: > On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: > > I had a short chat with Rob last night about this. I'm going to loop > > him in to the conversation, as he wrote the binding. > > > > > > When most of the other clocks that we deal with are being requested, > > > > they rely on being index zero: > > > > > > > > drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, > > > > NULL); > > > > > > Look at drivers/clk/clkdev.c, there's some fuzzy matching > > > involved when you pass NULL as connection id. > > > > Yes, I've been looking at that. This is why it works currently. I > > think I need to change all of the drivers to specify which clock they > > want. At the moment that 'fuzzy matching' is what's saving us. If > > anyone were to change our DTS file to match what the binding says, > > then it would cease to work. I'm guessing this is the same for all > > other DTS files too: > > I think if anything, the binding document(s) should be updated to > describe that apb_pclk is referred to by name, and the names of the > other clocks should be described in the specific device bindings. We can > then modify the drivers which grab clock 0 to explicitly grab the first > clock by name, and backwards compatibility should not be broken. > > I don't believe any other OS has implemented the common clock bindings, > and we've never supported the binding as described. Let's correct the > de-facto standard into a standard by decree. I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Fri, 23 Aug 2013, Mark Rutland wrote: On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too: I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken. I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree. I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote: On Fri, 23 Aug 2013, Mark Rutland wrote: On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too: I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken. I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree. I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices. Ideally, yes. However, we have to be careful to not break compatibility. I took a look at existing primecell drivers and what they do. The situation isn't so bad (with the exception of the half-dt/half-platform-code mess): * Some don't deal with clocks at all (no clk* in the driver). pl320 is in the ecx-common dtsi with only apb_pclk but has no binding defined. Some have no clocks defined in the dt and are presumably few clocks by platform data or are non-functional. I'm not sure how these DTs are going to be supported if and when we remove the platform data they depend upon. If we're really going to do that, then they are clearly not supported as-is long term. * The pl022 driver grabs the first clock to figure out the rate of the spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in the binding. The ste-u300 dts has two clock-names, apb_pclk and spi_clk (in that order), but they refer to the same clock. Given the existing driver simply grabs the first clock and they're both the same, we could re-order the names and make the driver grab the second clock. That wouldn't break backwards compatibility with the sole dts file we have using the binding, though this assumes no-one else has a dt lying around with different clocks. * pl010 grabs the first clock given to it to figure out the uart rate (assuming it is UARTCLK), but it's only in integratorap.dts, without clocks, and is presumably fed by platform data. There is no binding document. pl011 grabs the first clock given to figure out the UART rate (assuming it is UARTCLK). The binding explicitly states it's only given apb_pclk, despite UARTCLK and PCLK being separate inputs to the IP block. These two bindings don't describe the hardware, and should be fixed. The only way I can think to make this work without breaknig backwards compatibility would be to try to grab the second clock and then fall back to the first if there isn't one. The other option is to break backwards compatibility, but I'm not sure that's much of an option. * pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them. If you think those suggestions are OK, I can put together a series to fix this. Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 27 Aug 2013, Mark Rutland wrote: On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote: On Fri, 23 Aug 2013, Mark Rutland wrote: On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too: I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken. I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree. I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices. Ideally, yes. However, we have to be careful to not break compatibility. I took a look at existing primecell drivers and what they do. The situation isn't so bad (with the exception of the half-dt/half-platform-code mess): * Some don't deal with clocks at all (no clk* in the driver). pl320 is in the ecx-common dtsi with only apb_pclk but has no binding defined. Some have no clocks defined in the dt and are presumably few clocks by platform data or are non-functional. I'm not sure how these DTs are going to be supported if and when we remove the platform data they depend upon. If we're really going to do that, then they are clearly not supported as-is long term. * The pl022 driver grabs the first clock to figure out the rate of the spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in the binding. The ste-u300 dts has two clock-names, apb_pclk and spi_clk (in that order), but they refer to the same clock. Given the existing driver simply grabs the first clock and they're both the same, we could re-order the names and make the driver grab the second clock. That wouldn't break backwards compatibility with the sole dts file we have using the binding, though this assumes no-one else has a dt lying around with different clocks. * pl010 grabs the first clock given to it to figure out the uart rate (assuming it is UARTCLK), but it's only in integratorap.dts, without clocks, and is presumably fed by platform data. There is no binding document. pl011 grabs the first clock given to figure out the UART rate (assuming it is UARTCLK). The binding explicitly states it's only given apb_pclk, despite UARTCLK and PCLK being separate inputs to the IP block. These two bindings don't describe the hardware, and should be fixed. The only way I can think to make this work without breaknig backwards compatibility would be to try to grab the second clock and then fall back to the first if there isn't one. The other option is to break backwards compatibility, but I'm not sure that's much of an option. * pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them. If you think those suggestions are OK, I can put together a series to fix this. I think we need to hear from Rob before we proceed tbh, as he is the original author and should have a chance to voice his opinion. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 27, 2013 at 9:08 AM, Lee Jones lee.jo...@linaro.org wrote: On Tue, 27 Aug 2013, Mark Rutland wrote: On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote: On Fri, 23 Aug 2013, Mark Rutland wrote: On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too: I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken. I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree. I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices. Ideally, yes. However, we have to be careful to not break compatibility. I took a look at existing primecell drivers and what they do. The situation isn't so bad (with the exception of the half-dt/half-platform-code mess): * Some don't deal with clocks at all (no clk* in the driver). pl320 is in the ecx-common dtsi with only apb_pclk but has no binding defined. Some have no clocks defined in the dt and are presumably few clocks by platform data or are non-functional. I'm not sure how these DTs are going to be supported if and when we remove the platform data they depend upon. If we're really going to do that, then they are clearly not supported as-is long term. * The pl022 driver grabs the first clock to figure out the rate of the spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in the binding. The ste-u300 dts has two clock-names, apb_pclk and spi_clk (in that order), but they refer to the same clock. Given the existing driver simply grabs the first clock and they're both the same, we could re-order the names and make the driver grab the second clock. That wouldn't break backwards compatibility with the sole dts file we have using the binding, though this assumes no-one else has a dt lying around with different clocks. * pl010 grabs the first clock given to it to figure out the uart rate (assuming it is UARTCLK), but it's only in integratorap.dts, without clocks, and is presumably fed by platform data. There is no binding document. pl011 grabs the first clock given to figure out the UART rate (assuming it is UARTCLK). The binding explicitly states it's only given apb_pclk, despite UARTCLK and PCLK being separate inputs to the IP block. These two bindings don't describe the hardware, and should be fixed. The only way I can think to make this work without breaknig backwards compatibility would be to try to grab the second clock and then fall back to the first if there isn't one. The other option is to break backwards compatibility, but I'm not sure that's much of an option. This was an oversight since highbank has a single clock. But yes, this should be 2 clocks. It should be fixed and in a compatible way please. * pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them. I think this was waiting for some generic display bindings? Pawel may know. Rob -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 2013-08-27 at 16:51 +0100, Rob Herring wrote: * pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them. I think this was waiting for some generic display bindings? Pawel may know. Common Display Framework it was ;-) And yes, it was waiting for it, but it is getting bored with waiting so I'll cut off the CDF bits and post the rest this or next week. As to the APB clock, we've been there before... http://article.gmane.org/gmane.linux.ports.arm.kernel/188927 Cheers! Pawel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: > I had a short chat with Rob last night about this. I'm going to loop > him in to the conversation, as he wrote the binding. > > > > When most of the other clocks that we deal with are being requested, > > > they rely on being index zero: > > > > > > drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, NULL); > > > > Look at drivers/clk/clkdev.c, there's some fuzzy matching > > involved when you pass NULL as connection id. > > Yes, I've been looking at that. This is why it works currently. I > think I need to change all of the drivers to specify which clock they > want. At the moment that 'fuzzy matching' is what's saving us. If > anyone were to change our DTS file to match what the binding says, > then it would cease to work. I'm guessing this is the same for all > other DTS files too: I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken. I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree. Thanks, Mark. > > arch/arm/boot/dts/imx23.dtsi:clock-names = "uart", "apb_pclk"; > arch/arm/boot/dts/imx28.dtsi:clock-names = "uart", "apb_pclk"; > arch/arm/boot/dts/nspire-cx.dts: clock-names = "uart_clk", > "apb_pclk"; > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "timclk", > "apb_pclk"; > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "timclk", > "apb_pclk"; > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "rng", "apb_pclk"; > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "mclk", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "refclk", > "timclk", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "mclk", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "KMIREFCLK", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "KMIREFCLK", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "wdogclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "timclken1", > "timclken2", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "timclken1", > "timclken2", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "clcdclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "refclk", > "timclk", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "mclk", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "KMIREFCLK", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "KMIREFCLK", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "wdogclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "timclken1", > "timclken2", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "timclken1", > "timclken2", "apb_pclk"; > arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "clcdclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts: clock-names = "wdogclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = "clcdclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = "timclk", > "apb_pclk"; > arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = "wdogclk", > "apb_pclk"; > > -- > Lee Jones > Linaro ST-Ericsson Landing Team Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog > -- To unsubscribe from this list: send the line "unsubscribe
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding. > > When most of the other clocks that we deal with are being requested, > > they rely on being index zero: > > > > drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, NULL); > > Look at drivers/clk/clkdev.c, there's some fuzzy matching > involved when you pass NULL as connection id. Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too: arch/arm/boot/dts/imx23.dtsi:clock-names = "uart", "apb_pclk"; arch/arm/boot/dts/imx28.dtsi:clock-names = "uart", "apb_pclk"; arch/arm/boot/dts/nspire-cx.dts: clock-names = "uart_clk", "apb_pclk"; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "timclk", "apb_pclk"; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "timclk", "apb_pclk"; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "rng", "apb_pclk"; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = "mclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "refclk", "timclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "mclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "KMIREFCLK", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "KMIREFCLK", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "wdogclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "timclken1", "timclken2", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "timclken1", "timclken2", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = "clcdclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "refclk", "timclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "mclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "KMIREFCLK", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "KMIREFCLK", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "uartclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "wdogclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "timclken1", "timclken2", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "timclken1", "timclken2", "apb_pclk"; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = "clcdclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts: clock-names = "wdogclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = "clcdclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = "timclk", "apb_pclk"; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = "wdogclk", "apb_pclk"; -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too: arch/arm/boot/dts/imx23.dtsi:clock-names = uart, apb_pclk; arch/arm/boot/dts/imx28.dtsi:clock-names = uart, apb_pclk; arch/arm/boot/dts/nspire-cx.dts: clock-names = uart_clk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = timclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = timclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = rng, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = mclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = refclk, timclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = mclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = wdogclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = clcdclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = refclk, timclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = mclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = wdogclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = clcdclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts: clock-names = wdogclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = clcdclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = timclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = wdogclk, apb_pclk; -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote: I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too: I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken. I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree. Thanks, Mark. arch/arm/boot/dts/imx23.dtsi:clock-names = uart, apb_pclk; arch/arm/boot/dts/imx28.dtsi:clock-names = uart, apb_pclk; arch/arm/boot/dts/nspire-cx.dts: clock-names = uart_clk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = timclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = timclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = rng, apb_pclk; arch/arm/boot/dts/ste-nomadik-stn8815.dtsi: clock-names = mclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = refclk, timclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = mclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = wdogclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: clock-names = clcdclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = refclk, timclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = mclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = KMIREFCLK, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = uartclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = wdogclk, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = timclken1, timclken2, apb_pclk; arch/arm/boot/dts/vexpress-v2m.dtsi: clock-names = clcdclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts: clock-names = wdogclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = clcdclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = timclk, apb_pclk; arch/arm/boot/dts/vexpress-v2p-ca9.dts: clock-names = wdogclk, apb_pclk; -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Aug 22, 2013 at 04:41:16PM +0100, Lee Jones wrote: > > When most of the other clocks that we deal with are being requested, > they rely on being index zero: > > drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, NULL); Once a device has more than a single clock all clocks should have a connection id. clk_get(dev, NULL) should only be used for single clock devices. Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Aug 22, 2013 at 04:41:16PM +0100, Lee Jones wrote:
> On Thu, 22 Aug 2013, Mark Rutland wrote:
>
> > On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote:
> > > On Thu, 22 Aug 2013, Mark Rutland wrote:
> > >
> > > > On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > > > > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > > > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones
> > > > > > wrote:
> > > > > >
> > > > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > > > > @@ -572,6 +572,8 @@
> > > > > > > v-i2c-supply = <_vape_reg>;
> > > > > > >
> > > > > > > clock-frequency = <40>;
> > > > > > > + clocks = <_kclk 3 3>, <_pclk 3
> > > > > > > 3>;
> > > > > > > + clock-names = "nmk-i2c.0", "apb_pclk";
> > > > >
> > > > > Why do most clocks in this series have the instance number in the
> > > > > clock
> > > > > names? This looks very wrong to me.
> > > >
> > > > +1. The clock names should be the input names to the unit, they
> > > > shouldn't vary per instance.
> > >
> > > So I just had a quick look, and it looks like they each have their own
> > > clock:
> > >
> > > clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
> > > clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
> > > clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
> > > clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
> > > clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
> > > clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
> > > clk_register_clkdev(clk, NULL, "nmk-i2c.3");
> > >
> > > /* etc */
> > >
> > > When using the names in Device Tree it doesn't actually matter what
> > > you call the first clock. You can call it "fred" if you wanted and it
> > > would still work, but in light of the naming conventions above and the
> > > fact that each clock can all be controlled independently, do we still
> > > want to use the name of the parent clock i.e. i2cclk?
> >
> > Sorry, I don't follow.
> >
> >
> > The name should be the name of the clock _input_ on the block described,
> > as should be listed in documentation for the i2c block. The name should
> > not vary with instance, and the name should not (necessarily) match the
> > _output_ of the provider. Surely there's documentation for the i2c block
> > that gives a name for the clock input(s)?
>
> It's okay, I've fixed the patches.
Ok.
>
> Linus, branch fixed-up and pushed.
>
> > On a related note, I see that this doesn't follow the primecell clock
> > bindings, which seem to rely on "apb_pclk" being first in the list. I
> > see that other primecell device bindings don't follow that in dts or
> > drivers, so I'm not sure how to fix that brokenness.
>
> To which bindings do you refer? After taking a *quick* look, I see it
> being the other way around. Whenever the "apb_pclk" is requested, it
> is done so by name:
>
> drivers/amba/bus.c: struct clk *pclk = clk_get(>dev, "apb_pclk");
>
> So when clk_get() calls of_clk_get_by_name(), the clock-name index is
> returned correctly by of_property_match_string(), which then passes
> that index through of_clk_get() and does the right thing.
>
> When most of the other clocks that we deal with are being requested,
> they rely on being index zero:
>
> drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, NULL);
>
> At the moment this works for us, so I don't think we need to go
> around populating the name params, but we might have to if this falls
> apart in some way (probably likely if you 'fixed' whatever brokenness
> you're wanting to fix ;-) )
The brokenness here is that the binding document states one thing, and
drivers do precisely the opposite.
Documentation/devicetree/bindings/arm/primecell.txt states:
- clocks : From common clock binding. First clock is phandle to clock for apb
pclk. Additional clocks are optional and specific to those peripherals.
- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
Yet as you've pointed out above we don't care which index apb_pblk is,
but do for the other clock, which we get at index 0 (where the binding
doc states apb_pclk should be).
If all users are doing that currently, it's possible to fix up the
binding document. If not, then it's a mess that can't be corrected...
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, 22 Aug 2013, Mark Rutland wrote:
> On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote:
> > On Thu, 22 Aug 2013, Mark Rutland wrote:
> >
> > > On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > > > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones
> > > > > wrote:
> > > > >
> > > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > > > @@ -572,6 +572,8 @@
> > > > > > v-i2c-supply = <_vape_reg>;
> > > > > >
> > > > > > clock-frequency = <40>;
> > > > > > + clocks = <_kclk 3 3>, <_pclk 3 3>;
> > > > > > + clock-names = "nmk-i2c.0", "apb_pclk";
> > > >
> > > > Why do most clocks in this series have the instance number in the clock
> > > > names? This looks very wrong to me.
> > >
> > > +1. The clock names should be the input names to the unit, they
> > > shouldn't vary per instance.
> >
> > So I just had a quick look, and it looks like they each have their own
> > clock:
> >
> > clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
> > clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
> > clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
> > clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
> > clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
> > clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
> > clk_register_clkdev(clk, NULL, "nmk-i2c.3");
> >
> > /* etc */
> >
> > When using the names in Device Tree it doesn't actually matter what
> > you call the first clock. You can call it "fred" if you wanted and it
> > would still work, but in light of the naming conventions above and the
> > fact that each clock can all be controlled independently, do we still
> > want to use the name of the parent clock i.e. i2cclk?
>
> Sorry, I don't follow.
>
>
> The name should be the name of the clock _input_ on the block described,
> as should be listed in documentation for the i2c block. The name should
> not vary with instance, and the name should not (necessarily) match the
> _output_ of the provider. Surely there's documentation for the i2c block
> that gives a name for the clock input(s)?
It's okay, I've fixed the patches.
Linus, branch fixed-up and pushed.
> On a related note, I see that this doesn't follow the primecell clock
> bindings, which seem to rely on "apb_pclk" being first in the list. I
> see that other primecell device bindings don't follow that in dts or
> drivers, so I'm not sure how to fix that brokenness.
To which bindings do you refer? After taking a *quick* look, I see it
being the other way around. Whenever the "apb_pclk" is requested, it
is done so by name:
drivers/amba/bus.c: struct clk *pclk = clk_get(>dev, "apb_pclk");
So when clk_get() calls of_clk_get_by_name(), the clock-name index is
returned correctly by of_property_match_string(), which then passes
that index through of_clk_get() and does the right thing.
When most of the other clocks that we deal with are being requested,
they rely on being index zero:
drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(>dev, NULL);
At the moment this works for us, so I don't think we need to go
around populating the name params, but we might have to if this falls
apart in some way (probably likely if you 'fixed' whatever brokenness
you're wanting to fix ;-) )
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote:
> On Thu, 22 Aug 2013, Mark Rutland wrote:
>
> > On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote:
> > > >
> > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > > @@ -572,6 +572,8 @@
> > > > > v-i2c-supply = <_vape_reg>;
> > > > >
> > > > > clock-frequency = <40>;
> > > > > + clocks = <_kclk 3 3>, <_pclk 3 3>;
> > > > > + clock-names = "nmk-i2c.0", "apb_pclk";
> > >
> > > Why do most clocks in this series have the instance number in the clock
> > > names? This looks very wrong to me.
> >
> > +1. The clock names should be the input names to the unit, they
> > shouldn't vary per instance.
>
> So I just had a quick look, and it looks like they each have their own
> clock:
>
> clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
> clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
> clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
> clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
> clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
> clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
> clk_register_clkdev(clk, NULL, "nmk-i2c.3");
>
> /* etc */
>
> When using the names in Device Tree it doesn't actually matter what
> you call the first clock. You can call it "fred" if you wanted and it
> would still work, but in light of the naming conventions above and the
> fact that each clock can all be controlled independently, do we still
> want to use the name of the parent clock i.e. i2cclk?
Sorry, I don't follow.
The name should be the name of the clock _input_ on the block described,
as should be listed in documentation for the i2c block. The name should
not vary with instance, and the name should not (necessarily) match the
_output_ of the provider. Surely there's documentation for the i2c block
that gives a name for the clock input(s)?
On a related note, I see that this doesn't follow the primecell clock
bindings, which seem to rely on "apb_pclk" being first in the list. I
see that other primecell device bindings don't follow that in dts or
drivers, so I'm not sure how to fix that brokenness.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, 22 Aug 2013, Mark Rutland wrote:
> On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote:
> > >
> > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > @@ -572,6 +572,8 @@
> > > > v-i2c-supply = <_vape_reg>;
> > > >
> > > > clock-frequency = <40>;
> > > > + clocks = <_kclk 3 3>, <_pclk 3 3>;
> > > > + clock-names = "nmk-i2c.0", "apb_pclk";
> >
> > Why do most clocks in this series have the instance number in the clock
> > names? This looks very wrong to me.
>
> +1. The clock names should be the input names to the unit, they
> shouldn't vary per instance.
So I just had a quick look, and it looks like they each have their own
clock:
clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
clk_register_clkdev(clk, NULL, "nmk-i2c.3");
/* etc */
When using the names in Device Tree it doesn't actually matter what
you call the first clock. You can call it "fred" if you wanted and it
would still work, but in light of the naming conventions above and the
fact that each clock can all be controlled independently, do we still
want to use the name of the parent clock i.e. i2cclk?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, 22 Aug 2013, Mark Rutland wrote: > On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: > > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: > > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote: > > > > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi > > > > @@ -572,6 +572,8 @@ > > > > v-i2c-supply = <_vape_reg>; > > > > > > > > clock-frequency = <40>; > > > > + clocks = <_kclk 3 3>, <_pclk 3 3>; > > > > + clock-names = "nmk-i2c.0", "apb_pclk"; > > > > Why do most clocks in this series have the instance number in the clock > > names? This looks very wrong to me. > > +1. The clock names should be the input names to the unit, they > shouldn't vary per instance. Duly noted. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote: > > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi > > > @@ -572,6 +572,8 @@ > > > v-i2c-supply = <_vape_reg>; > > > > > > clock-frequency = <40>; > > > + clocks = <_kclk 3 3>, <_pclk 3 3>; > > > + clock-names = "nmk-i2c.0", "apb_pclk"; > > Why do most clocks in this series have the instance number in the clock > names? This looks very wrong to me. +1. The clock names should be the input names to the unit, they shouldn't vary per instance. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 20 Aug 2013, Linus Walleij wrote: > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote: > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi > > @@ -572,6 +572,8 @@ > > v-i2c-supply = <_vape_reg>; > > > > clock-frequency = <40>; > > + clocks = <_kclk 3 3>, <_pclk 3 3>; > > + clock-names = "nmk-i2c.0", "apb_pclk"; > > To avoid confusing the clock name "nmk-i2c.0" with the device > name in Linux of that device instance, can we use a name such > that it is clear that this is not a dev_name match? Once we've settled the other thread, I'll look into renaming this something less Linuxy. > "i2c0" works just fine as name I think? Sure. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 20 Aug 2013, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; To avoid confusing the clock name nmk-i2c.0 with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match? Once we've settled the other thread, I'll look into renaming this something less Linuxy. i2c0 works just fine as name I think? Sure. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. +1. The clock names should be the input names to the unit, they shouldn't vary per instance. Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, 22 Aug 2013, Mark Rutland wrote: On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. +1. The clock names should be the input names to the unit, they shouldn't vary per instance. Duly noted. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, 22 Aug 2013, Mark Rutland wrote: On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. +1. The clock names should be the input names to the unit, they shouldn't vary per instance. So I just had a quick look, and it looks like they each have their own clock: clk = clk_reg_prcc_kclk(p1_i2c1_kclk, i2cclk, clkrst1_base, BIT(2), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p1_i2c2_kclk, i2cclk, clkrst1_base, BIT(6), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p2_i2c3_kclk, i2cclk, clkrst2_base, BIT(0), CLK_SET_RATE_GATE); clk_register_clkdev(clk, NULL, nmk-i2c.3); /* etc */ When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it fred if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk? -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote: On Thu, 22 Aug 2013, Mark Rutland wrote: On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. +1. The clock names should be the input names to the unit, they shouldn't vary per instance. So I just had a quick look, and it looks like they each have their own clock: clk = clk_reg_prcc_kclk(p1_i2c1_kclk, i2cclk, clkrst1_base, BIT(2), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p1_i2c2_kclk, i2cclk, clkrst1_base, BIT(6), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p2_i2c3_kclk, i2cclk, clkrst2_base, BIT(0), CLK_SET_RATE_GATE); clk_register_clkdev(clk, NULL, nmk-i2c.3); /* etc */ When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it fred if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk? Sorry, I don't follow. The name should be the name of the clock _input_ on the block described, as should be listed in documentation for the i2c block. The name should not vary with instance, and the name should not (necessarily) match the _output_ of the provider. Surely there's documentation for the i2c block that gives a name for the clock input(s)? On a related note, I see that this doesn't follow the primecell clock bindings, which seem to rely on apb_pclk being first in the list. I see that other primecell device bindings don't follow that in dts or drivers, so I'm not sure how to fix that brokenness. Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, 22 Aug 2013, Mark Rutland wrote: On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote: On Thu, 22 Aug 2013, Mark Rutland wrote: On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. +1. The clock names should be the input names to the unit, they shouldn't vary per instance. So I just had a quick look, and it looks like they each have their own clock: clk = clk_reg_prcc_kclk(p1_i2c1_kclk, i2cclk, clkrst1_base, BIT(2), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p1_i2c2_kclk, i2cclk, clkrst1_base, BIT(6), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p2_i2c3_kclk, i2cclk, clkrst2_base, BIT(0), CLK_SET_RATE_GATE); clk_register_clkdev(clk, NULL, nmk-i2c.3); /* etc */ When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it fred if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk? Sorry, I don't follow. The name should be the name of the clock _input_ on the block described, as should be listed in documentation for the i2c block. The name should not vary with instance, and the name should not (necessarily) match the _output_ of the provider. Surely there's documentation for the i2c block that gives a name for the clock input(s)? It's okay, I've fixed the patches. Linus, branch fixed-up and pushed. On a related note, I see that this doesn't follow the primecell clock bindings, which seem to rely on apb_pclk being first in the list. I see that other primecell device bindings don't follow that in dts or drivers, so I'm not sure how to fix that brokenness. To which bindings do you refer? After taking a *quick* look, I see it being the other way around. Whenever the apb_pclk is requested, it is done so by name: drivers/amba/bus.c: struct clk *pclk = clk_get(pcdev-dev, apb_pclk); So when clk_get() calls of_clk_get_by_name(), the clock-name index is returned correctly by of_property_match_string(), which then passes that index through of_clk_get() and does the right thing. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); At the moment this works for us, so I don't think we need to go around populating the name params, but we might have to if this falls apart in some way (probably likely if you 'fixed' whatever brokenness you're wanting to fix ;-) ) -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Aug 22, 2013 at 04:41:16PM +0100, Lee Jones wrote: On Thu, 22 Aug 2013, Mark Rutland wrote: On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote: On Thu, 22 Aug 2013, Mark Rutland wrote: On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote: On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. +1. The clock names should be the input names to the unit, they shouldn't vary per instance. So I just had a quick look, and it looks like they each have their own clock: clk = clk_reg_prcc_kclk(p1_i2c1_kclk, i2cclk, clkrst1_base, BIT(2), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p1_i2c2_kclk, i2cclk, clkrst1_base, BIT(6), CLK_SET_RATE_GATE); clk = clk_reg_prcc_kclk(p2_i2c3_kclk, i2cclk, clkrst2_base, BIT(0), CLK_SET_RATE_GATE); clk_register_clkdev(clk, NULL, nmk-i2c.3); /* etc */ When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it fred if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk? Sorry, I don't follow. The name should be the name of the clock _input_ on the block described, as should be listed in documentation for the i2c block. The name should not vary with instance, and the name should not (necessarily) match the _output_ of the provider. Surely there's documentation for the i2c block that gives a name for the clock input(s)? It's okay, I've fixed the patches. Ok. Linus, branch fixed-up and pushed. On a related note, I see that this doesn't follow the primecell clock bindings, which seem to rely on apb_pclk being first in the list. I see that other primecell device bindings don't follow that in dts or drivers, so I'm not sure how to fix that brokenness. To which bindings do you refer? After taking a *quick* look, I see it being the other way around. Whenever the apb_pclk is requested, it is done so by name: drivers/amba/bus.c: struct clk *pclk = clk_get(pcdev-dev, apb_pclk); So when clk_get() calls of_clk_get_by_name(), the clock-name index is returned correctly by of_property_match_string(), which then passes that index through of_clk_get() and does the right thing. When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); At the moment this works for us, so I don't think we need to go around populating the name params, but we might have to if this falls apart in some way (probably likely if you 'fixed' whatever brokenness you're wanting to fix ;-) ) The brokenness here is that the binding document states one thing, and drivers do precisely the opposite. Documentation/devicetree/bindings/arm/primecell.txt states: - clocks : From common clock binding. First clock is phandle to clock for apb pclk. Additional clocks are optional and specific to those peripherals. - clock-names : From common clock binding. Shall be apb_pclk for first clock. Yet as you've pointed out above we don't care which index apb_pblk is, but do for the other clock, which we get at index 0 (where the binding doc states apb_pclk should be). If all users are doing that currently, it's possible to fix up the binding document. If not, then it's a mess that can't be corrected... Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Aug 22, 2013 at 04:41:16PM +0100, Lee Jones wrote: When most of the other clocks that we deal with are being requested, they rely on being index zero: drivers/i2c/busses/i2c-nomadik.c: dev-clk = clk_get(adev-dev, NULL); Once a device has more than a single clock all clocks should have a connection id. clk_get(dev, NULL) should only be used for single clock devices. Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Wed, Aug 21, 2013 at 10:28 AM, Lee Jones wrote: > On Tue, 20 Aug 2013, Linus Walleij wrote: >> On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote: >> >> > +++ b/arch/arm/boot/dts/dbx5x0.dtsi >> > @@ -572,6 +572,8 @@ >> > v-i2c-supply = <_vape_reg>; >> > >> > clock-frequency = <40>; >> > + clocks = <_kclk 3 3>, <_pclk 3 3>; >> > + clock-names = "nmk-i2c.0", "apb_pclk"; >> >> To avoid confusing the clock name "nmk-i2c.0" with the device >> name in Linux of that device instance, can we use a name such >> that it is clear that this is not a dev_name match? >> >> "i2c0" works just fine as name I think? > > If you do that, then I think you need to change all of these too: > > git grep -e "\.0" -e "\.1" -e "\.2" -e "\.3" -- drivers/clk/ux500/ No, as I said in some other patch, all these clk_register_clkdev()s are unused in device tree boots and should not even be executed on the DT boot path. (But maybe you've proven me wrong there in this other thread ...) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 20 Aug 2013, Linus Walleij wrote: > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote: > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi > > @@ -572,6 +572,8 @@ > > v-i2c-supply = <_vape_reg>; > > > > clock-frequency = <40>; > > + clocks = <_kclk 3 3>, <_pclk 3 3>; > > + clock-names = "nmk-i2c.0", "apb_pclk"; > > To avoid confusing the clock name "nmk-i2c.0" with the device > name in Linux of that device instance, can we use a name such > that it is clear that this is not a dev_name match? > > "i2c0" works just fine as name I think? If you do that, then I think you need to change all of these too: git grep -e "\.0" -e "\.1" -e "\.2" -e "\.3" -- drivers/clk/ux500/ -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, 20 Aug 2013, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; To avoid confusing the clock name nmk-i2c.0 with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match? i2c0 works just fine as name I think? If you do that, then I think you need to change all of these too: git grep -e \.0 -e \.1 -e \.2 -e \.3 -- drivers/clk/ux500/ -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Wed, Aug 21, 2013 at 10:28 AM, Lee Jones lee.jo...@linaro.org wrote: On Tue, 20 Aug 2013, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; To avoid confusing the clock name nmk-i2c.0 with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match? i2c0 works just fine as name I think? If you do that, then I think you need to change all of these too: git grep -e \.0 -e \.1 -e \.2 -e \.3 -- drivers/clk/ux500/ No, as I said in some other patch, all these clk_register_clkdev()s are unused in device tree boots and should not even be executed on the DT boot path. (But maybe you've proven me wrong there in this other thread ...) Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote: > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi > > @@ -572,6 +572,8 @@ > > v-i2c-supply = <_vape_reg>; > > > > clock-frequency = <40>; > > + clocks = <_kclk 3 3>, <_pclk 3 3>; > > + clock-names = "nmk-i2c.0", "apb_pclk"; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones wrote: > +++ b/arch/arm/boot/dts/dbx5x0.dtsi > @@ -572,6 +572,8 @@ > v-i2c-supply = <_vape_reg>; > > clock-frequency = <40>; > + clocks = <_kclk 3 3>, <_pclk 3 3>; > + clock-names = "nmk-i2c.0", "apb_pclk"; To avoid confusing the clock name "nmk-i2c.0" with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match? "i2c0" works just fine as name I think? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; To avoid confusing the clock name nmk-i2c.0 with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match? i2c0 works just fine as name I think? Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote: On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones lee.jo...@linaro.org wrote: +++ b/arch/arm/boot/dts/dbx5x0.dtsi @@ -572,6 +572,8 @@ v-i2c-supply = db8500_vape_reg; clock-frequency = 40; + clocks = prcc_kclk 3 3, prcc_pclk 3 3; + clock-names = nmk-i2c.0, apb_pclk; Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
Signed-off-by: Lee Jones
---
arch/arm/boot/dts/dbx5x0.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index d682ea0..e49c679 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -572,6 +572,8 @@
v-i2c-supply = <_vape_reg>;
clock-frequency = <40>;
+ clocks = <_kclk 3 3>, <_pclk 3 3>;
+ clock-names = "nmk-i2c.0", "apb_pclk";
};
i2c@80122000 {
@@ -585,6 +587,9 @@
v-i2c-supply = <_vape_reg>;
clock-frequency = <40>;
+
+ clocks = <_kclk 1 2>, <_pclk 1 2>;
+ clock-names = "nmk-i2c.1", "apb_pclk";
};
i2c@80128000 {
@@ -598,6 +603,9 @@
v-i2c-supply = <_vape_reg>;
clock-frequency = <40>;
+
+ clocks = <_kclk 1 6>, <_pclk 1 6>;
+ clock-names = "nmk-i2c.2", "apb_pclk";
};
i2c@8011 {
@@ -611,6 +619,9 @@
v-i2c-supply = <_vape_reg>;
clock-frequency = <40>;
+
+ clocks = <_kclk 2 0>, <_pclk 2 0>;
+ clock-names = "nmk-i2c.3", "apb_pclk";
};
i2c@8012a000 {
@@ -624,6 +635,9 @@
v-i2c-supply = <_vape_reg>;
clock-frequency = <40>;
+
+ clocks = <_kclk 1 9>, <_pclk 1 9>;
+ clock-names = "nmk-i2c.4", "apb_pclk";
};
ssp@80002000 {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
arch/arm/boot/dts/dbx5x0.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index d682ea0..e49c679 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -572,6 +572,8 @@
v-i2c-supply = db8500_vape_reg;
clock-frequency = 40;
+ clocks = prcc_kclk 3 3, prcc_pclk 3 3;
+ clock-names = nmk-i2c.0, apb_pclk;
};
i2c@80122000 {
@@ -585,6 +587,9 @@
v-i2c-supply = db8500_vape_reg;
clock-frequency = 40;
+
+ clocks = prcc_kclk 1 2, prcc_pclk 1 2;
+ clock-names = nmk-i2c.1, apb_pclk;
};
i2c@80128000 {
@@ -598,6 +603,9 @@
v-i2c-supply = db8500_vape_reg;
clock-frequency = 40;
+
+ clocks = prcc_kclk 1 6, prcc_pclk 1 6;
+ clock-names = nmk-i2c.2, apb_pclk;
};
i2c@8011 {
@@ -611,6 +619,9 @@
v-i2c-supply = db8500_vape_reg;
clock-frequency = 40;
+
+ clocks = prcc_kclk 2 0, prcc_pclk 2 0;
+ clock-names = nmk-i2c.3, apb_pclk;
};
i2c@8012a000 {
@@ -624,6 +635,9 @@
v-i2c-supply = db8500_vape_reg;
clock-frequency = 40;
+
+ clocks = prcc_kclk 1 9, prcc_pclk 1 9;
+ clock-names = nmk-i2c.4, apb_pclk;
};
ssp@80002000 {
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
