SAMA5D2 Slow audio on i2s1. Unable to select GCKL for i2s1 on SFR_I2SCKLSEL from devicetree

This forum is for users of Microchip MPUs and who are interested in using Linux OS.

Moderator: nferre

egrande
Posts: 2
Joined: Thu May 16, 2019 3:01 pm

SAMA5D2 Slow audio on i2s1. Unable to select GCKL for i2s1 on SFR_I2SCKLSEL from devicetree

Thu May 16, 2019 8:56 pm

I have a custom board based on sama5d2 and I am having some issues related to the i2s1 bus.

Our board has two codecs connected to i2s0 and i2s1.
Everything seemed to be working but I have realized that audio played with codec1 is too slow.
I have measured the clock signals on i2s0 and i2s1 and the clocks provided on i2s1 are 10% slower (40kHz, instead of 44kHz in WS signal).

I have reviewed the i2s clock generation diagram (sama2d2 manual, figure 44-3, page 1260) and I the issue is that the configuration in SFR_I2SCLKSEL (0xF8030090) is different for both i2s buses.
On that register, the GCKL clock is selected for i2s0 but PCKL is selected for i2s1 (SFR_I2SCLKSEL -> 0x1).

I have modified the SFR_I2SCLKSEL register to 0x3 (to select GCLK for both i2s) while playing audio on codec1 and then the clocks on both codecs are equal, and the audio is played at normal speed. Great!

The issue I am facing now is that I am not able to configure this clock selection in the devicetree.
I am using the official linux-4.14.73.linux-at91-linux4sam_6.0, and I am using an overlay for the sama5d2.dtsi:

Code: Select all

#include "sama5d2.dtsi"
[...]
&i2s0 {
  pinctrl-names = "default";
  pinctrl-0 = <&pinctrl_i2s0_default>;
  status = "okay";
};
&i2s1 {
  pinctrl-names = "default";
  pinctrl-0 = <&pinctrl_i2s1_default>;
  status = "okay";
};
The sama5d2.dtsi file has these configuration related to the i2s buses:

Code: Select all

i2s0: i2s@f8050000 {
    compatible = "atmel,sama5d2-i2s";
    reg = <0xf8050000 0x100>;
    interrupts = <54 IRQ_TYPE_LEVEL_HIGH 7>;
    dmas = <&dma0
            (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) |
             AT91_XDMAC_DT_PERID(31))>,
           <&dma0
            (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) |
             AT91_XDMAC_DT_PERID(32))>;
    dma-names = "tx", "rx";
    clocks = <&i2s0_clk>, <&i2s0_gclk>;
    clock-names = "pclk", "gclk";
    assigned-clocks = <&i2s0muxck>;
    assigned-clock-parents = <&i2s0_gclk>;
    status = "disabled";
};

i2s1: i2s@fc04c000 {
    compatible = "atmel,sama5d2-i2s";
    reg = <0xfc04c000 0x100>;
    interrupts = <55 IRQ_TYPE_LEVEL_HIGH 7>;
    dmas = <&dma0
            (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) |
             AT91_XDMAC_DT_PERID(33))>,
           <&dma0
            (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) |
             AT91_XDMAC_DT_PERID(34))>;
    dma-names = "tx", "rx";
    clocks = <&i2s1_clk>, <&i2s1_gclk>;
    clock-names = "pclk", "gclk";
    assigned-clocks = <&i2s1muxck>;
    assigned-parrents = <&i2s1_gclk>;
    status = "disabled";
};
I have noticed that there is an error on the i2s1 definition:
"assigned-parrents = <&i2s1_gclk>;" shall be "assigned-clock-parents = <&i2s1_gclk>;"

I have fixed this on my overlay:

Code: Select all

#include "sama5d2.dtsi"
[...]
&i2s0 {
  pinctrl-names = "default";
  pinctrl-0 = <&pinctrl_i2s0_default>;
  status = "okay";
};
&i2s1 {
  pinctrl-names = "default";
  pinctrl-0 = <&pinctrl_i2s1_default>;
  status = "okay";
  // Fix error in sama5d2.dtsi
  /delete-property/ assigned-parrents;
  assigned-clock-parents = <&i2s1_gclk>;
};
But the issue persist: the SFR_I2SCLKSEL is 0x01 instead of 0x03.

The sama5d2.dti also contains:

Code: Select all

i2s_clkmux {
  compatible = "atmel,sama5d2-clk-i2s-mux";
  #address-cells = <1>;
  #size-cells = <0>;

  i2s0muxck: i2s0_muxclk {
	  clocks = <&i2s0_clk>, <&i2s0_gclk>;
	  #clock-cells = <0>;
	  reg = <0>;
  };

  i2s1muxck: i2s1_muxclk {
	  clocks = <&i2s1_clk>, <&i2s1_gclk>;
	  #clock-cells = <0>;
	  reg = <1>;
  };
};

So I have included the muxclk clock on my overlay:

Code: Select all

&i2s0 {
  pinctrl-names = "default";
  pinctrl-0 = <&pinctrl_i2s0_default>;
  status = "okay";
  
  // Add muxclk
  clocks = <&i2s0_clk>, <&i2s0_gclk>, <&i2s0muxck>;
  clock-names = "pclk", "gclk", "muxclk";
  
  assigned-clock-parents = <&i2s0_gclk>;
};
&i2s1 {
  pinctrl-names = "default";
  pinctrl-0 = <&pinctrl_i2s1_default>;
  status = "okay";
  
  // Add muxclk
  clocks = <&i2s1_clk>, <&i2s1_gclk>, <&i2s1muxck>;
  clock-names = "pclk", "gclk", "muxclk";
  
  // Fix error in sama5d2.dtsi
  /delete-property/ assigned-parrents;
  assigned-clock-parents = <&i2s1_gclk>;
};
Same results.

The issue is that the "asigned-clock-parents" seems not to be working on i2s1.

Am I missing something?
Thank you in advance.
Last edited by egrande on Wed May 29, 2019 7:21 am, edited 1 time in total.
blue_z
Location: USA
Posts: 1940
Joined: Thu Apr 19, 2007 10:15 pm

Re: SAMA5D2 Slow audio on i2s1. Unable to select GCKL for i2s1 on SFR_I2SCKLSEL from devicetree

Thu May 16, 2019 11:40 pm

Take a look at Documentation/devicetree/bindings/clock/clock-bindings.txt for the definition of the "assigned-clock-parents" property,
The assigned-clock-parents property should contain a list of parent clocks in the form of a phandle and clock specifier pair and
the assigned-clock-rates property should contain a list of frequencies in Hz.
Both these properties should correspond to the clocks listed in the assigned-clocks property.
as well as Documentation/devicetree/bindings/sound/atmel-i2s.txt.

Consider inserting printk() statements (for example in drivers/clk/clk-conf.c and/or drivers/clk/at91/clk-i2s-mux.c) to trace what is going on.


Regards
oohay0000
Posts: 12
Joined: Fri Mar 20, 2009 10:56 pm

Re: SAMA5D2 Slow audio on i2s1. Unable to select GCKL for i2s1 on SFR_I2SCKLSEL from devicetree

Mon May 27, 2019 11:57 pm

Sorry for hijacking the thread. I have a SAMA2D5 board connected to a WM8904 through I2S1 that I can't not get it to work. May I ask what audio chip do you use? Can you post your device tree node for the sound device? My sound device wouldn't register if I use

Code: Select all

		atmel,ssc-controller = <&i2s1>;


The sound driver will register if I change the line to

Code: Select all

		 atmel,ssc-controller = <&ssc1>;


But the play back application will crash.

Thank you very much in advance.
egrande
Posts: 2
Joined: Thu May 16, 2019 3:01 pm

[PATCH] Re: SAMA5D2 Slow audio on i2s1. Unable to select GCKL for i2s1 on SFR_I2SCKLSEL from devicetree

Mon Jun 10, 2019 1:12 pm

There is an issue parsing the i2s_clkmux defined in the sama5d2.dtsi devicetree:

Code: Select all

i2s_clkmux { 
compatible = "atmel,sama5d2-clk-i2s-mux";
  #address-cells = <1>;
  #size-cells = <0>;
  i2s0muxck: i2s0_muxclk {
    clocks = <&i2s0_clk>, <&i2s0_gclk>;
    #clock-cells = <0>;
    reg = <0>;
  };
  i2s1muxck: i2s1_muxclk {
    clocks = <&i2s1_clk>, <&i2s1_gclk>;
    #clock-cells = <0>;
    reg = <1>;
  };
};
In drivers/clk/at91/clk-i2s-mux.c, the "of_sama5d2_clk_i2s_mux_setup" function reads each i2sXmuxck node, where "reg" is the i2s bus number and "clocks" lists the available clocks.
This function uses "of_property_read_u8" to read the "reg" value, but it always reads 0.
The issue is that "of_property_read_u8" calls "of_property_read_u8_array" and, according to the function description in include/linux/of.h:

Code: Select all

/**
 * of_property_read_u8_array - Find and read an array of u8 from a property.
 *
 [...]
 *
 * dts entry of array should be like:
 *	property = /bits/ 8 <0x50 0x60 0x70>;
 *
 * The out_values is modified only if a valid u8 value can be decoded.
 */
So the "reg" lines in i2s_clkmux should be defined as "reg = /bits/ 8 <X>;", which is not the case.

The following patch fixes the issue using "of_property_read_u32" instead of "of_property_read_u8":

Code: Select all

diff --git a/linux-4.14.73.linux-at91-linux4sam_6.0/drivers/clk/at91/clk-i2s-mux.c b/linux-4.14.73.linux-at91-linux4sam_6.0/drivers/  clk/at91/clk-i2s-mux.c
  index f0c3c30..7aef6c1 100644
  --- a/linux-4.14.73.linux-at91-linux4sam_6.0/drivers/clk/at91/clk-i2s-mux.c
  +++ b/linux-4.14.73.linux-at91-linux4sam_6.0/drivers/clk/at91/clk-i2s-mux.c
  @@ -82,7 +82,7 @@ at91_clk_i2s_mux_register(struct regmap *regmap, const char *name,
   static void __init of_sama5d2_clk_i2s_mux_setup(struct device_node *np)
   {
    struct regmap *regmap_sfr;
  - u8 bus_id;
  + u32 bus_id;
    const char *parent_names[2];
    struct device_node *i2s_mux_np;
    struct clk_hw *hw;
  @@ -93,7 +93,7 @@ static void __init of_sama5d2_clk_i2s_mux_setup(struct device_node *np)
      return;
  
    for_each_child_of_node(np, i2s_mux_np) {
  -   if (of_property_read_u8(i2s_mux_np, "reg", &bus_id))
  +   if (of_property_read_u32(i2s_mux_np, "reg", &bus_id))
        continue;
  
      if (bus_id > I2S_BUS_NR)
  @@ -103,8 +103,8 @@ static void __init of_sama5d2_clk_i2s_mux_setup(struct device_node *np)
      if (ret != 2)
        continue;
  
      hw = at91_clk_i2s_mux_register(regmap_sfr, i2s_mux_np->name,
  -                parent_names, 2, bus_id);
  +                parent_names, 2, (u8)bus_id);
      if (IS_ERR(hw))
        continue;
  
      if (bus_id > I2S_BUS_NR)
With this patch we can select "gclk" for both i2s buses.

Return to “LINUX”

Who is online

Users browsing this forum: Bing [Bot] and 4 guests