| Message ID | 1330375973-10681-16-git-send-email-sjg@chromium.org |
|---|---|
| State | New, archived |
| Headers | show |
On 02/27/2012 01:52 PM, Simon Glass wrote: > A common requirement is to find the clock ID for a peripheral. This is the > second cell of the 'clocks' property (the first being the phandle itself). > +int clock_decode_periph_id(const void *blob, int node) > + valid = clock_periph_id_isvalid(id); clock_periph_id_isvalid() is not the correct function to use here; the code should be checking for invalid IDs in the CAR binding, not invalid IDs in the HW periph ID definition. They're different. Just to be explicit, the function you need here would be: int clkid_to_periphid(int clkid) { if (clk_id > 95) return -1; switch (clk_id) { case 1: case 2: case 7: case 10: case 20: case 30: case 35: case 49: case 56: case 74: case 77: case 78: case 79: case 80: case 81: case 82: case 83: case 91: case 95: return -1; default: return clkid; } }
Hi Stephen, On Mon, Feb 27, 2012 at 3:41 PM, Stephen Warren <swarren@nvidia.com> wrote: > On 02/27/2012 01:52 PM, Simon Glass wrote: >> A common requirement is to find the clock ID for a peripheral. This is the >> second cell of the 'clocks' property (the first being the phandle itself). > >> +int clock_decode_periph_id(const void *blob, int node) > >> + valid = clock_periph_id_isvalid(id); > > clock_periph_id_isvalid() is not the correct function to use here; the > code should be checking for invalid IDs in the CAR binding, not invalid > IDs in the HW periph ID definition. They're different. > > Just to be explicit, the function you need here would be: > > int clkid_to_periphid(int clkid) > { > if (clk_id > 95) > return -1; > switch (clk_id) { > case 1: > case 2: > case 7: > case 10: > case 20: > case 30: > case 35: > case 49: > case 56: > case 74: > case 77: > case 78: > case 79: > case 80: > case 81: > case 82: > case 83: > case 91: > case 95: > return -1; > default: > return clkid; > } > } Ick. Why is 7 in there, and did you miss 76? Also U-Boot only goes up to 88 at present so should I change the first test to match? Regards, Simon > > -- > nvpublic > -- > To unsubscribe from this list: send the line "unsubscribe linux-tegra" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Simon Glass wrote at Tuesday, February 28, 2012 10:46 AM: > On Mon, Feb 27, 2012 at 3:41 PM, Stephen Warren <swarren@nvidia.com> wrote: > > On 02/27/2012 01:52 PM, Simon Glass wrote: > >> A common requirement is to find the clock ID for a peripheral. This is the > >> second cell of the 'clocks' property (the first being the phandle itself). > > > >> +int clock_decode_periph_id(const void *blob, int node) > > > >> + valid = clock_periph_id_isvalid(id); > > > > clock_periph_id_isvalid() is not the correct function to use here; the > > code should be checking for invalid IDs in the CAR binding, not invalid > > IDs in the HW periph ID definition. They're different. > > > > Just to be explicit, the function you need here would be: > > > > int clkid_to_periphid(int clkid) > > { > > if (clk_id > 95) > > return -1; > > switch (clk_id) { > > case 1: > > case 2: > > case 7: > > case 10: > > case 20: > > case 30: > > case 35: > > case 49: > > case 56: > > case 74: > > case 77: > > case 78: > > case 79: > > case 80: > > case 81: > > case 82: > > case 83: > > case 91: > > case 95: > > return -1; > > default: > > return clkid; > > } > > } > > Ick. > > Why is 7 in there, 7 affects both the UART2 and VFIR clocks/blocks. > and did you miss 76? No, that's the undocumented "la" clock. > Also U-Boot only goes up to 88 > at present so should I change the first test to match? No, clocks 89, 90, 92, 93, and 94 are defined in the binding, which matches the CLK_OUT_ENB registers in the Tegra CAR HW (albeit not the CLK_RST registers, since there are some differences between the two).
Hi Stephen, On Tue, Feb 28, 2012 at 10:37 AM, Stephen Warren <swarren@nvidia.com> wrote: > Simon Glass wrote at Tuesday, February 28, 2012 10:46 AM: >> On Mon, Feb 27, 2012 at 3:41 PM, Stephen Warren <swarren@nvidia.com> wrote: >> > On 02/27/2012 01:52 PM, Simon Glass wrote: >> >> A common requirement is to find the clock ID for a peripheral. This is the >> >> second cell of the 'clocks' property (the first being the phandle itself). >> > >> >> +int clock_decode_periph_id(const void *blob, int node) >> > >> >> + valid = clock_periph_id_isvalid(id); >> > >> > clock_periph_id_isvalid() is not the correct function to use here; the >> > code should be checking for invalid IDs in the CAR binding, not invalid >> > IDs in the HW periph ID definition. They're different. >> > >> > Just to be explicit, the function you need here would be: >> > >> > int clkid_to_periphid(int clkid) >> > { >> > if (clk_id > 95) >> > return -1; >> > switch (clk_id) { >> > case 1: >> > case 2: >> > case 7: >> > case 10: >> > case 20: >> > case 30: >> > case 35: >> > case 49: >> > case 56: >> > case 74: >> > case 77: >> > case 78: >> > case 79: >> > case 80: >> > case 81: >> > case 82: >> > case 83: >> > case 91: >> > case 95: >> > return -1; >> > default: >> > return clkid; >> > } >> > } >> >> Ick. >> >> Why is 7 in there, > > 7 affects both the UART2 and VFIR clocks/blocks. > >> and did you miss 76? > > No, that's the undocumented "la" clock. > >> Also U-Boot only goes up to 88 >> at present so should I change the first test to match? > > No, clocks 89, 90, 92, 93, and 94 are defined in the binding, which > matches the CLK_OUT_ENB registers in the Tegra CAR HW (albeit not the > CLK_RST registers, since there are some differences between the two). For both of your comments, since they aren't used in U-Boot, wouldn't it be more correct to flag these as errors also? We would have to update at least the clock.h header to support them. Regards, Simon > > -- > nvpublic >
Simon Glass wrote at Tuesday, February 28, 2012 11:44 AM: > On Tue, Feb 28, 2012 at 10:37 AM, Stephen Warren <swarren@nvidia.com> wrote: > > Simon Glass wrote at Tuesday, February 28, 2012 10:46 AM: > >> On Mon, Feb 27, 2012 at 3:41 PM, Stephen Warren <swarren@nvidia.com> wrote: > >> > On 02/27/2012 01:52 PM, Simon Glass wrote: > >> >> A common requirement is to find the clock ID for a peripheral. This is the > >> >> second cell of the 'clocks' property (the first being the phandle itself). > >> > > >> >> +int clock_decode_periph_id(const void *blob, int node) > >> > > >> >> + valid = clock_periph_id_isvalid(id); > >> > > >> > clock_periph_id_isvalid() is not the correct function to use here; the > >> > code should be checking for invalid IDs in the CAR binding, not invalid > >> > IDs in the HW periph ID definition. They're different. > >> > > >> > Just to be explicit, the function you need here would be: > >> > > >> > int clkid_to_periphid(int clkid) > >> > { > >> > if (clk_id > 95) > >> > return -1; > >> > switch (clk_id) { > >> > case 1: > >> > case 2: > >> > case 7: > >> > case 10: > >> > case 20: > >> > case 30: > >> > case 35: > >> > case 49: > >> > case 56: > >> > case 74: > >> > case 77: > >> > case 78: > >> > case 79: > >> > case 80: > >> > case 81: > >> > case 82: > >> > case 83: > >> > case 91: > >> > case 95: > >> > return -1; > >> > default: > >> > return clkid; > >> > } > >> > } > >> > >> Ick. > >> > >> Why is 7 in there, > > > > 7 affects both the UART2 and VFIR clocks/blocks. > > > >> and did you miss 76? > > > > No, that's the undocumented "la" clock. > > > >> Also U-Boot only goes up to 88 > >> at present so should I change the first test to match? > > > > No, clocks 89, 90, 92, 93, and 94 are defined in the binding, which > > matches the CLK_OUT_ENB registers in the Tegra CAR HW (albeit not the > > CLK_RST registers, since there are some differences between the two). > > For both of your comments, since they aren't used in U-Boot, wouldn't > it be more correct to flag these as errors also? We would have to > update at least the clock.h header to support them. It's probably more correct to update the periph_id enum to define all the valid values, but either way is fine.
Hi Stephen, On Tue, Feb 28, 2012 at 10:51 AM, Stephen Warren <swarren@nvidia.com> wrote: > Simon Glass wrote at Tuesday, February 28, 2012 11:44 AM: >> On Tue, Feb 28, 2012 at 10:37 AM, Stephen Warren <swarren@nvidia.com> wrote: >> > Simon Glass wrote at Tuesday, February 28, 2012 10:46 AM: >> >> On Mon, Feb 27, 2012 at 3:41 PM, Stephen Warren <swarren@nvidia.com> wrote: >> >> > On 02/27/2012 01:52 PM, Simon Glass wrote: >> >> >> A common requirement is to find the clock ID for a peripheral. This is the >> >> >> second cell of the 'clocks' property (the first being the phandle itself). >> >> > >> >> >> +int clock_decode_periph_id(const void *blob, int node) >> >> > >> >> >> + valid = clock_periph_id_isvalid(id); >> >> > >> >> > clock_periph_id_isvalid() is not the correct function to use here; the >> >> > code should be checking for invalid IDs in the CAR binding, not invalid >> >> > IDs in the HW periph ID definition. They're different. >> >> > >> >> > Just to be explicit, the function you need here would be: >> >> > >> >> > int clkid_to_periphid(int clkid) >> >> > { >> >> > if (clk_id > 95) >> >> > return -1; >> >> > switch (clk_id) { >> >> > case 1: >> >> > case 2: >> >> > case 7: >> >> > case 10: >> >> > case 20: >> >> > case 30: >> >> > case 35: >> >> > case 49: >> >> > case 56: >> >> > case 74: >> >> > case 77: >> >> > case 78: >> >> > case 79: >> >> > case 80: >> >> > case 81: >> >> > case 82: >> >> > case 83: >> >> > case 91: >> >> > case 95: >> >> > return -1; >> >> > default: >> >> > return clkid; >> >> > } >> >> > } >> >> >> >> Ick. >> >> >> >> Why is 7 in there, >> > >> > 7 affects both the UART2 and VFIR clocks/blocks. >> > >> >> and did you miss 76? >> > >> > No, that's the undocumented "la" clock. >> > >> >> Also U-Boot only goes up to 88 >> >> at present so should I change the first test to match? >> > >> > No, clocks 89, 90, 92, 93, and 94 are defined in the binding, which >> > matches the CLK_OUT_ENB registers in the Tegra CAR HW (albeit not the >> > CLK_RST registers, since there are some differences between the two). >> >> For both of your comments, since they aren't used in U-Boot, wouldn't >> it be more correct to flag these as errors also? We would have to >> update at least the clock.h header to support them. > > It's probably more correct to update the periph_id enum to define all > the valid values, but either way is fine. OK - BTW what does 'LA' stand for? Regards, Simon > > -- > nvpublic >
Simon Glass wrote at Tuesday, February 28, 2012 4:50 PM: > On Tue, Feb 28, 2012 at 10:51 AM, Stephen Warren <swarren@nvidia.com> wrote: > > Simon Glass wrote at Tuesday, February 28, 2012 11:44 AM: > >> On Tue, Feb 28, 2012 at 10:37 AM, Stephen Warren <swarren@nvidia.com> wrote: ... > >> >> and did you miss 76? > >> > > >> > No, that's the undocumented "la" clock. .... > OK - BTW what does 'LA' stand for? Sorry, I don't know; it's not documented. I just confirmed it exists.
diff --git a/arch/arm/cpu/armv7/tegra2/clock.c b/arch/arm/cpu/armv7/tegra2/clock.c index 11d2346..ffbfc28 100644 --- a/arch/arm/cpu/armv7/tegra2/clock.c +++ b/arch/arm/cpu/armv7/tegra2/clock.c @@ -28,6 +28,7 @@ #include <asm/arch/tegra2.h> #include <common.h> #include <div64.h> +#include <fdtdec.h> /* * This is our record of the current clock rate of each clock. We don't @@ -918,6 +919,24 @@ void clock_ll_start_uart(enum periph_id periph_id) reset_set_enable(periph_id, 0); } + +int clock_decode_periph_id(const void *blob, int node) +{ + enum periph_id id; + int err, valid; + u32 cell[2]; + + err = fdtdec_get_int_array(blob, node, "clocks", cell, + ARRAY_SIZE(cell)); + if (err) + return -1; + id = cell[1]; + + valid = clock_periph_id_isvalid(id); + assert(valid); + return valid ? id : PERIPH_ID_NONE; +} + int clock_verify(void) { struct clk_pll *pll = get_pll(CLOCK_ID_PERIPH); diff --git a/arch/arm/include/asm/arch-tegra2/clock.h b/arch/arm/include/asm/arch-tegra2/clock.h index 080ef18..6b12c76 100644 --- a/arch/arm/include/asm/arch-tegra2/clock.h +++ b/arch/arm/include/asm/arch-tegra2/clock.h @@ -177,6 +177,7 @@ enum periph_id { PERIPH_ID_CRAM2, PERIPH_ID_COUNT, + PERIPH_ID_NONE = -1, }; /* Converts a clock number to a clock register: 0=L, 1=H, 2=U */ @@ -355,6 +356,18 @@ unsigned clock_get_rate(enum clock_id clkid); */ void clock_ll_start_uart(enum periph_id periph_id); +/** + * Decode a peripheral ID from a device tree node. + * + * This works by looking up the peripheral's 'clocks' node and reading out + * the second cell, which is the clock number / peripheral ID. + * + * @param blob FDT blob to use + * @param node Node to look at + * @return peripheral ID, or PERIPH_ID_NONE if none + */ +enum periph_id clock_decode_periph_id(const void *blob, int node); + /* * Checks that clocks are valid and prints a warning if not *
A common requirement is to find the clock ID for a peripheral. This is the second cell of the 'clocks' property (the first being the phandle itself). Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v4: - Add fdtdec function to return peripheral ID Changes in v6: - Move peripheral decode function into Tegra's clock.c arch/arm/cpu/armv7/tegra2/clock.c | 19 +++++++++++++++++++ arch/arm/include/asm/arch-tegra2/clock.h | 13 +++++++++++++ 2 files changed, 32 insertions(+), 0 deletions(-)