Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935033AbbKTITX (ORCPT ); Fri, 20 Nov 2015 03:19:23 -0500 Received: from mail-io0-f172.google.com ([209.85.223.172]:33640 "EHLO mail-io0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934844AbbKTITR (ORCPT ); Fri, 20 Nov 2015 03:19:17 -0500 MIME-Version: 1.0 In-Reply-To: References: <1447940410-9728-1-git-send-email-jacob@teenage.engineering> From: Jacob Siverskog Date: Fri, 20 Nov 2015 09:18:57 +0100 Message-ID: Subject: Re: [PATCH] clk: si5351: Add setup steps To: Belisko Marek Cc: Michael Turquette , Stephen Boyd , Sebastian Hesselbarth , LKML , linux-clk@vger.kernel.org, Jens Rudberg Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4114 Lines: 109 Hi Marek! Thanks for your feedback. On Thu, Nov 19, 2015 at 3:18 PM, Belisko Marek wrote: > Hi Jacob, > > On Thu, Nov 19, 2015 at 2:40 PM, Jacob Siverskog > wrote: >> This is according to figure 12 ("I2C Programming Procedure") in >> "Si5351A/B/C Data Sheet" >> (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). >> >> Without the PLL soft reset, we were unable to get three outputs >> working at the same time. >> >> According to Silicon Labs support, performing PLL soft reset will only >> be noticable if the PLL parameters have been changed. >> >> Signed-off-by: Jacob Siverskog >> Signed-off-by: Jens Rudberg >> --- >> drivers/clk/clk-si5351.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c >> index e346b22..110de35 100644 >> --- a/drivers/clk/clk-si5351.c >> +++ b/drivers/clk/clk-si5351.c >> @@ -1091,6 +1091,11 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate, >> si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, >> SI5351_CLK_POWERDOWN, 0); >> >> + /* do a pll soft reset on both plls, needed in some cases to get all >> + * outputs running */ > ^^^^ wrong format of multi line comment, please look at > Documentation/CodingStyle >> + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, >> + SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); > ^^^^ according datasheet PLL_RESET_A/B are self clearing bits > valid for Si5351A/C only > so probably we need to add some code for B model Well, this is interesting. In the latest data sheet (that I've been reading, https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf), this is not mentioned at all. For the register descriptions, it only refers to "AN619 Generating a register map" (http://www.silabs.com/Support%20Documents/TechnicalDocs/AN619.pdf), where no such caveat exists. I found an old datasheet on my computer that has the same phrasing as yours. I have sent an e-mail to our SiLabs representative regarding this matter. >> + >> dev_dbg(&hwdata->drvdata->client->dev, >> "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", >> __func__, clk_hw_get_name(hw), (1 << rdiv), >> @@ -1359,6 +1364,14 @@ static int si5351_i2c_probe(struct i2c_client *client, >> return PTR_ERR(drvdata->regmap); >> } >> >> + /* Disable outputs */ >> + si5351_reg_write(drvdata, SI5351_OUTPUT_ENABLE_CTRL, 0xff); >> + >> + /* Power down all output drivers */ >> + for (n = 0; n < 8; n++) { >> + si5351_reg_write(drvdata, SI5351_CLK0_CTRL + n, 0x80); >> + } > ^^^ single line after for statement doesn't need curly > braces (if you run ./scripts/check_patch it will complain) I will fix all embarrassing cosmetic issues for the next version of the patch. >> + >> /* Disable interrupts */ >> si5351_reg_write(drvdata, SI5351_INTERRUPT_MASK, 0xf0); >> /* Ensure pll select is on XTAL for Si5351A/B */ >> -- >> 2.6.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > > BR, > > marek > > -- > as simple and primitive as possible > ------------------------------------------------- > Marek Belisko - OPEN-NANDRA > Freelance Developer > > Ruska Nova Ves 219 | Presov, 08005 Slovak Republic > Tel: +421 915 052 184 > skype: marekwhite > twitter: #opennandra > web: http://open-nandra.com Thanks, Jacob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/