From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sebastian Hesselbarth Subject: Re: [PATCH v7] clk: add si5351 i2c common clock driver Date: Wed, 10 Apr 2013 18:34:01 +0200 Message-ID: References: <1365439617-18816-1-git-send-email-sebastian.hesselbarth@gmail.com> <1365586810-6752-1-git-send-email-sebastian.hesselbarth@gmail.com> <51653C3E.2020702@gmail.com> <14854575.YAd2OMKOy9@ganymedes> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7982864754365401494==" Return-path: In-Reply-To: <14854575.YAd2OMKOy9@ganymedes> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Michal Bachraty Cc: linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Lars-Peter Clausen , Stephen Warren , Mike Turquette , Rabeeh Khoury , Guenter Roeck , Pawel Moll , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Rob Herring , Dom Cobley , Russell King - ARM Linux , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Jean-Francois Moine , Mark Brown , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrew Morton List-Id: devicetree@vger.kernel.org --===============7982864754365401494== Content-Type: multipart/alternative; boundary=e89a8f646a07cb263004da043cd1 --e89a8f646a07cb263004da043cd1 Content-Type: text/plain; charset=ISO-8859-1 On Wed, Apr 10, 2013 at 4:48 PM, Michal Bachraty < michal.bachraty-6oiIBCxl0MMjD8S081q9vkEOCMrvLtNR@public.gmane.org> wrote: > Hi Sebastian, > This driver doesn't work for me. In my case, u-boot initializes si-5351 and > power down unused clocks while booting kernel. there is need for power up > clocks as was in previous versions of your driver. > See patch, whre the problem is fixed: > > @@ -992,6 +992,10 @@ static long si5351_clkout_round_rate(struct clk_hw > *hw, > unsigned long rate, > } while (1); > } > rate = *parent_rate >> rdiv; > + > + /* powerup clkout */ > + si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, > + SI5351_CLK_POWERDOWN, 0); > > dev_dbg(&hwdata->drvdata->client->dev, > "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", > > With this lines, driver works well. > Hmm, is there any driver using the clock output? Does it clk_prepare_enable() the clock? I tend not to mess with anything the bootloader or eeprom config left disabled. It works for me, but here the driver will prepare/enable the clock prior use. > Also, > > > > +==Example== > > > + > > > +/* 25MHz reference crystal */ > > > +ref25: ref25M { > > > + compatible = "fixed-clock"; > > > + #clock-cells = <0>; > > > + clock-frequency = <25000000>; > > > +}; > > > + > > > +i2c-master-node { > > > + > > > + /* Si5351a msop10 i2c clock generator */ > > > + si5351a: clock-generator@60 { > > > + compatible = "silabs,si5351a-msop"; > > > + reg = <0x60>; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + #clock-cells = <1>; > > > + > > > + /* connect xtal input to 25MHz reference */ > > > + clocks = <&ref25>; > > > + > > > + /* connect xtal input as source of pll0 and pll1 */ > > > + silabs,pll-source = <0 0>, <1 0>; > > > + > > > + /* > > > + * overwrite clkout0 configuration with: > > > + * - 8mA output drive strength > > > + * - pll0 as clock source of multisynth0 > > > + * - multisynth0 as clock source of output divider > > > + * - multisynth0 can change pll0 > > > + * - set initial clock frequency of 74.25MHz > > > + */ > > > + clkout0 { > > > + reg = <0>; > > > + silabs,drive-strength = <8>; > > > + silabs,multisynth-source = <0>; > > > + silabs,clock-source = <0>; > > > + silabs,pll-master; > > > + clock-frequency = <74250000>; > > > + }; > > > + > > > + /* > > > + * overwrite clkout1 configuration with: > > > + * - 4mA output drive strength > > > + * - pll1 as clock source of multisynth1 > > > + * - multisynth1 as clock source of output divider > > > + * - multisynth1 can change pll1 > > > + */ > > > + clkout1 { > > > + reg = <1>; > > > + silabs,drive-strength = <4>; > > > + silabs,multisynth-source = <1>; > > > + silabs,clock-source = <0>; > > > + pll-master; > > > + }; > > > + > Is definition of pll-master in clkout1 correct? should not be silabs,pll- > master ? > Good catch. Sebastian --e89a8f646a07cb263004da043cd1 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On W= ed, Apr 10, 2013 at 4:48 PM, Michal Bachraty <michal.ba= chraty-6oiIBCxl0MMjD8S081q9vkEOCMrvLtNR@public.gmane.org> wrote:
Hi Sebastian,
This driver doesn't work for me. In my case, u-boot initializes si-5351= and
power down unused clocks while booting kernel. =A0there is need for power u= p
clocks as was in previous versions of your driver.
See patch, whre the problem is fixed:

@@ -992,6 +992,10 @@ static long si5351_clkout_round_rate(struct clk_hw *hw= ,
unsigned long rate,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } while (1);
=A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0 rate =3D *parent_rate >> rdiv;
+
+ =A0 =A0 =A0 /* powerup clkout */
+ =A0 =A0 =A0 si5351_set_bits(hwdata->drvdata, SI5351_= CLK0_CTRL + hwdata->num,
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 SI5351_CLK_POWERDOWN, 0);

=A0 =A0 =A0 =A0 dev_dbg(&hwdata->drvdata->client->dev,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "%s - %s: rdiv =3D %= u, parent_rate =3D %lu, rate =3D %lu\n",

With this lines, driver works well.

Hmm, is there any driver using the clock output? Does it clk_prepare_ena= ble() the clock?
I tend not to mess with anything the bootloa= der or eeprom config left disabled. It works
for me, but here the driver will prepare/enable the clock prior use.
=A0
Also,

> > +=3D=3DExample=3D=3D
> > +
> > +/* 25MHz reference crystal */
> > +ref25: ref25M {
> > + =A0 compatible =3D "fixed-clock";
> > + =A0 #clock-cells =3D <0>;
> > + =A0 clock-frequency =3D <25000000>;
> > +};
> > +
> > +i2c-master-node {
> > +
> > + =A0 /* Si5351a msop10 i2c clock generator */
> > + =A0 si5351a: clock-generator@60 {
> > + =A0 =A0 =A0 =A0 =A0 compatible =3D "silabs,si5351a-msop&qu= ot;;
> > + =A0 =A0 =A0 =A0 =A0 reg =3D <0x60>;
> > + =A0 =A0 =A0 =A0 =A0 #address-cells =3D <1>;
> > + =A0 =A0 =A0 =A0 =A0 #size-cells =3D <0>;
> > + =A0 =A0 =A0 =A0 =A0 #clock-cells =3D <1>;
> > +
> > + =A0 =A0 =A0 =A0 =A0 /* connect xtal input to 25MHz reference */=
> > + =A0 =A0 =A0 =A0 =A0 clocks =3D <&ref25>;
> > +
> > + =A0 =A0 =A0 =A0 =A0 /* connect xtal input as source of pll0 and= pll1 */
> > + =A0 =A0 =A0 =A0 =A0 silabs,pll-source =3D <0 0>, <1 0&= gt;;
> > +
> > + =A0 =A0 =A0 =A0 =A0 /*
> > + =A0 =A0 =A0 =A0 =A0 =A0* overwrite clkout0 configuration with:<= br> > > + =A0 =A0 =A0 =A0 =A0 =A0* - 8mA output drive strength
> > + =A0 =A0 =A0 =A0 =A0 =A0* - pll0 as clock source of multisynth0<= br> > > + =A0 =A0 =A0 =A0 =A0 =A0* - multisynth0 as clock source of outpu= t divider
> > + =A0 =A0 =A0 =A0 =A0 =A0* - multisynth0 can change pll0
> > + =A0 =A0 =A0 =A0 =A0 =A0* - set initial clock frequency of 74.25= MHz
> > + =A0 =A0 =A0 =A0 =A0 =A0*/
> > + =A0 =A0 =A0 =A0 =A0 clkout0 {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 silabs,drive-strength =3D &= lt;8>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 silabs,multisynth-source = =3D <0>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 silabs,clock-source =3D <= ;0>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 silabs,pll-master;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock-frequency =3D <742= 50000>;
> > + =A0 =A0 =A0 =A0 =A0 };
> > +
> > + =A0 =A0 =A0 =A0 =A0 /*
> > + =A0 =A0 =A0 =A0 =A0 =A0* overwrite clkout1 configuration with:<= br> > > + =A0 =A0 =A0 =A0 =A0 =A0* - 4mA output drive strength
> > + =A0 =A0 =A0 =A0 =A0 =A0* - pll1 as clock source of multisynth1<= br> > > + =A0 =A0 =A0 =A0 =A0 =A0* - multisynth1 as clock source of outpu= t divider
> > + =A0 =A0 =A0 =A0 =A0 =A0* - multisynth1 can change pll1
> > + =A0 =A0 =A0 =A0 =A0 =A0*/
> > + =A0 =A0 =A0 =A0 =A0 clkout1 {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <1>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 silabs,drive-strength =3D &= lt;4>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 silabs,multisynth-source = =3D <1>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 silabs,clock-source =3D <= ;0>;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pll-master;
> > + =A0 =A0 =A0 =A0 =A0 };
> > +
=A0Is definition of pll-master in clkout1 correct? should not b= e silabs,pll-
master ?

Good ca= tch.

Sebastian
--e89a8f646a07cb263004da043cd1-- --===============7982864754365401494== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss --===============7982864754365401494==--