from:"Sergey Suloev"

sun6i MIPI DSI driver EoTp

2019-03-11 Thread Sergey Suloev


Hi, Maxime,

according to MIPI DSI spec chapter 8.8.2  the EoTp packet can be enabled 
or disabled on the protocol level.
It seems like that it might be required for the panel I am working on 
(Himax HX8379A-based panel).
Here is some crappy driver I found on github showing that it might be an 
issue  [1]


May I ask you if this feature supported by sun6i MIPI DSI driver and 
where ?
As there is no datasheet available, could you please help to support 
this feature (if not yet) ?


The second point is that the panel requires NON-BURST-WITH-SYNC-EVENTS 
mode.

Could it be a problem with the current driver?

[1] 
https://github.com/lnfamous/Kernel_Htc_Pico_CyanogenMod12.1/blob/ff5eed4b86f5c9e653ebde4d89fe969c6abf611a/drivers/video/msm/mipi_hx8379a_video_wvga_pt.c#L107


[2] 
https://github.com/lnfamous/Kernel_Htc_Pico_CyanogenMod12.1/blob/ff5eed4b86f5c9e653ebde4d89fe969c6abf611a/drivers/video/msm/mipi_hx8379a_video_wvga_pt.c#L92


Thank you,
Sergey

Compilation issue with Linaro ARM toolchain

2021-01-18 Thread Sergey Suloev


Hi, guys,

I am having an issue builduing kernel 5.11 (rc4) with Linaro ARM 
toolchain. The issue seems to be related to CC plugins sources.
Here is my build log: https://pastebin.com/DTn7Szax. I have never seen 
this before with versions 5.10 and below.


Thank you,
Sergey

Re: Compilation issue with Linaro ARM toolchain

2021-01-18 Thread Sergey Suloev


Hi, Maxim,

thank you for the reply.
Installing libmpc-dev helped.

On 18.01.2021 19:04, Maxim Kuvyrkov wrote:

On 18 Jan 2021, at 18:59, Mathieu Poirier  wrote:

Adding the Linaro toolchain group mailing list.

On Mon, Jan 18, 2021 at 05:49:39PM +0300, Sergey Suloev wrote:

Hi, guys,

I am having an issue builduing kernel 5.11 (rc4) with Linaro ARM toolchain.
The issue seems to be related to CC plugins sources.
Here is my build log: https://pastebin.com/DTn7Szax. I have never seen this
before with versions 5.10 and below.

Thanks, Mathieu.

Hi Sergey,

MPC (along with GMP and MPFR) are libraries that GCC and its plugins depend on. 
 I think, installing libmpc-dev or equivalent should fix the problem; 
installing an equivalent of “apt build-dep gcc” might be more robust.

Linux kernel has recently relaxed the way it detects support for compiler 
plugins, so it’s now enabling them more often than before.

Hope this helps,

--
Maxim Kuvyrkov
https://www.linaro.org


Thanks,
Sergey

Re: [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents

2018-05-18 Thread Sergey Suloev


Hi, guys,

On 05/18/2018 05:46 PM, Jernej Škrabec wrote:

Hi,

Dne petek, 18. maj 2018 ob 12:01:16 CEST je Maxime Ripard napisal(a):

On Fri, May 18, 2018 at 03:15:22PM +0530, Jagan Teki wrote:

From: Jernej Skrabec 

Some SoCs with DW HDMI have multiple possible clock parents, like A64
and R40.

Expand HDMI PHY clock driver to support second clock parent.

Signed-off-by: Jernej Skrabec 
Signed-off-by: Jagan Teki 
---
Changes for v2:
- new patch

  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h  |  9 ++-
  drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 33 ---
  drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 89
  ++ 3 files changed, 96 insertions(+), 35
  deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 79154f0f674a..303189d6602c
100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -98,7 +98,8 @@

  #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN   BIT(29)
  #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN   BIT(28)
  #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33  BIT(27)

-#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL   BIT(26)
+#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK   BIT(26)
+#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26

  #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25)
  #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x)   ((x) << 22)
  #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x)((x) << 20)

@@ -146,7 +147,7 @@

  struct sun8i_hdmi_phy;
  
  struct sun8i_hdmi_phy_variant {


-   bool has_phy_clk;
+   int  phy_clk_num;

void (*phy_init)(struct sun8i_hdmi_phy *phy);
void (*phy_disable)(struct dw_hdmi *hdmi,

struct sun8i_hdmi_phy *phy);

@@ -160,6 +161,7 @@ struct sun8i_hdmi_phy {

struct clk  *clk_mod;
struct clk  *clk_phy;
struct clk  *clk_pll0;

+   struct clk  *clk_pll1;

unsigned intrcal;
struct regmap   *regs;
struct reset_control*rst_phy;

@@ -188,6 +190,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
*hdmi);

  void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
  const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);

-int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev);
+int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev,
+int clk_num);

  #endif /* _SUN8I_DW_HDMI_H_ */

diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index 5a52fc489a9d..0eadf087fc46
100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi
*hdmi,>
regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,

   SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);

-   regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
+   /*
+* NOTE: We have to be careful not to overwrite PHY parent
+* clock selection bit and clock divider.
+*/
+   regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
+  (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
+  pll_cfg1_init);

regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,

   (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
   pll_cfg2_init);

@@ -232,7 +238,7 @@ static int sun8i_hdmi_phy_config(struct dw_hdmi *hdmi,
void *data,>
regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,

   SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);

-   if (phy->variant->has_phy_clk)
+   if (phy->variant->phy_clk_num)

clk_set_rate(phy->clk_phy, mode->crtc_clock * 1000);

return phy->variant->phy_config(hdmi, phy, mode->crtc_clock * 1000);

@@ -393,7 +399,7 @@ static const struct sun8i_hdmi_phy_variant
sun8i_a83t_hdmi_phy = {>
  };
  
  static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {


-   .has_phy_clk = true,
+   .phy_clk_num = 1,

.phy_init = &sun8i_hdmi_phy_init_h3,
.phy_disable = &sun8i_hdmi_phy_disable_h3,
.phy_config = &sun8i_hdmi_phy_config_h3,

@@ -464,7 +470,7 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
struct device_node *node)>
goto err_put_clk_bus;

}

-   if (phy->variant->has_phy_clk) {
+   if (phy->variant->phy_clk_num) {

phy->clk_pll0 = of_clk_get_by_name(node, "pll-0");
if (IS_ERR(phy->clk_pll0)) {

dev_err(dev, "Could not get pll-0 clock\n");

@@ -472,7 +478,16 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
struct device_node *node)>

Re: [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents

2018-05-18 Thread Sergey Suloev


Hi, Jernej,

On 05/18/2018 06:15 PM, Jernej Škrabec wrote:

Hi,

Dne petek, 18. maj 2018 ob 17:09:40 CEST je Sergey Suloev napisal(a):

Hi, guys,

On 05/18/2018 05:46 PM, Jernej Škrabec wrote:

Hi,

Dne petek, 18. maj 2018 ob 12:01:16 CEST je Maxime Ripard napisal(a):

On Fri, May 18, 2018 at 03:15:22PM +0530, Jagan Teki wrote:

From: Jernej Skrabec 

Some SoCs with DW HDMI have multiple possible clock parents, like A64
and R40.

Expand HDMI PHY clock driver to support second clock parent.

Signed-off-by: Jernej Skrabec 
Signed-off-by: Jagan Teki 
---
Changes for v2:
- new patch

   drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h  |  9 ++-
   drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 33 ---
   drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 89
   ++ 3 files changed, 96 insertions(+), 35
   deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 79154f0f674a..303189d6602c
100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -98,7 +98,8 @@

   #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN  BIT(29)
   #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN  BIT(28)
   #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27)

-#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL   BIT(26)
+#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK   BIT(26)
+#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26

   #define SUN8I_HDMI_PHY_PLL_CFG1_PLLENBIT(25)
   #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x)  ((x) << 22)
   #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x)   ((x) << 20)

@@ -146,7 +147,7 @@

   struct sun8i_hdmi_phy;
   
   struct sun8i_hdmi_phy_variant {


-   bool has_phy_clk;
+   int  phy_clk_num;

void (*phy_init)(struct sun8i_hdmi_phy *phy);
void (*phy_disable)(struct dw_hdmi *hdmi,

struct sun8i_hdmi_phy *phy);

@@ -160,6 +161,7 @@ struct sun8i_hdmi_phy {

struct clk  *clk_mod;
struct clk  *clk_phy;
struct clk  *clk_pll0;

+   struct clk  *clk_pll1;

unsigned intrcal;
struct regmap   *regs;
struct reset_control*rst_phy;

@@ -188,6 +190,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
*hdmi);

   void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
   const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);

-int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device
*dev);
+int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device
*dev,
+int clk_num);

   #endif /* _SUN8I_DW_HDMI_H_ */

diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index
5a52fc489a9d..0eadf087fc46
100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi
*hdmi,>

regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,

   SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);

-   regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
+   /*
+* NOTE: We have to be careful not to overwrite PHY parent
+* clock selection bit and clock divider.
+*/
+   regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
+  (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
+  pll_cfg1_init);

regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,

   (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
   pll_cfg2_init);

@@ -232,7 +238,7 @@ static int sun8i_hdmi_phy_config(struct dw_hdmi
*hdmi,
void *data,>

regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,

   SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);

-   if (phy->variant->has_phy_clk)
+   if (phy->variant->phy_clk_num)

clk_set_rate(phy->clk_phy, mode->crtc_clock * 1000);

return phy->variant->phy_config(hdmi, phy, mode->crtc_clock * 1000);

@@ -393,7 +399,7 @@ static const struct sun8i_hdmi_phy_variant
sun8i_a83t_hdmi_phy = {>

   };
   
   static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {


-   .has_phy_clk = true,
+   .phy_clk_num = 1,

.phy_init = &sun8i_hdmi_phy_init_h3,
.phy_disable = &sun8i_hdmi_phy_disable_h3,
.phy_config = &sun8i_hdmi_phy_config_h3,

@@ -464,7 +470,7 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
struct device_node *node)>

goto err_put_clk_bus;

}

-   if (phy->variant->has_phy_clk) {
+   if (phy->variant->phy_clk_num) {

p

Re: [PATCH 7/7] ARM: dts: sun7i: Add dts file for the A20-linova1-7 HMI

2018-05-04 Thread Sergey Suloev

Hi, Giulio,

On 05/05/2018 12:52 AM, Giulio Benetti wrote:

Hi Maxime!

Il 04/05/2018 10:06, Maxime Ripard ha scritto:

Hi,

On Wed, May 02, 2018 at 06:41:34PM +0200, Giulio Benetti wrote:

You don't have to handcode the fragments anymore with the new syntax,
and U-Boot makes it really trivial to use if you use the FIT image
format to have multiple overlays bundled in the same image. You can
choose to apply them dynamically, for example based on an EEPROM or
some other metric to see which combination you have.

Ah, this is interesting. I'm going to experiment with that.

I'm struggling against this, I don't really know how to proceed,
except keeping monolithic dts files including other dtsi files.

About dt-overlays I've tried to look around lot of time,
but the only thing I've found is this:
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/arch/arm/boot/dts?h=topic/renesas-overlays

where they use .dtso tagging them as "/plugin/;"
and compile all .dtso found in dts folder.
Then they obtain .dtbo files that should be the dt-overlays we have
spoken

about right?

Yes. You don't have to do that though, you can just rely on dtc to
compile them, outside of the linux build system.

What I can't understand is if there's a real standard at this time to
follow, because on renesas-driver they use their way to handle all
.dtso

files, but on mainline there seems to be nothing about that.

I'm not sure what you mean here. It's just fragments of device tree,
that have to be compiled using dtc, that's it. You can use the Linux
build system infrastructure to do that, or you can build your own
simpler one. That's really up to you. See for example
https://github.com/NextThingCo/CHIP-dt-overlays/blob/master/Makefile

(even though the overlays themselves use the legacy syntax and
shouldn't really be used an examples)

Everything works now!
Thank you very much!
I've setted up a Repo on Github to give an example on how make it work
with no pain:

https://github.com/micronovasrl/linova-dtoverlays

At the moment it's a mess all around, but it's working and give an
idea on how to make it work. Though I'm going to clean it up well as a
base for linova dtoverlays.

Ah, btw, can you confirm me that base dts file must be compiled
outside kernel with:

dtc -@
Otherwise as in-tree dts with make dtbs "-@" argument is not passed.
Right?

Thank you a lot for your help and time again!

Best regards!

I'd highly recommend you to look at Armbian overlay support: it is easy
and elegant.

https://github.com/armbian/build/blob/master/patch/kernel/sunxi-next/add-overlay-compilation-support.patch

https://github.com/armbian/build/blob/master/patch/kernel/sunxi-next/add-sunxi-overlays.patch

Sergey

Re: [PATCH 0/3] Fix broken bananapi m2 devicetree/regulators

2018-02-10 Thread Sergey Suloev


On 02/09/2018 08:52 PM, Philipp Rossak wrote:

This patchseries fixes the bananapi m1 devicetree, to be able to boot again.
The first two patches update/improve the devicetree and the last patch adds
all missing regulators.

Regards,
Philipp

Philipp Rossak (3):
   arm: dts: sun6i: a31s: bpi-m2: update mmc supply nodes
   arm: dts: sun6i: a31s: bpi-m2: improve pmic properties
   arm: dts: sun6i: a31s: fix: bpi-m2: add missing regulators

  arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts | 70 +++-
  1 file changed, 67 insertions(+), 3 deletions(-)


patches are not working

Thanks

Re: [PATCH 0/3] Fix broken bananapi m2 devicetree/regulators

2018-02-10 Thread Sergey Suloev


On 02/11/2018 12:01 AM, Philipp Rossak wrote:

Hey Sergey,

Thanks for mentioning, but I think the problem has nothing to do with 
those patches. I tested them with the v4.15.0 Kernel since this is the 
last stable release and we are right now in the merging window.


I tested the latest mainline, without those patches and the kernel is 
not booting (I can't see any uart output).


Thanks,
Philipp

On 10.02.2018 14:56, Sergey Suloev wrote:

On 02/09/2018 08:52 PM, Philipp Rossak wrote:
This patchseries fixes the bananapi m1 devicetree, to be able to 
boot again.
The first two patches update/improve the devicetree and the last 
patch adds

all missing regulators.

Regards,
Philipp

Philipp Rossak (3):
   arm: dts: sun6i: a31s: bpi-m2: update mmc supply nodes
   arm: dts: sun6i: a31s: bpi-m2: improve pmic properties
   arm: dts: sun6i: a31s: fix: bpi-m2: add missing regulators

  arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts | 70 
+++-

  1 file changed, 67 insertions(+), 3 deletions(-)


patches are not working

Thanks


same problem, but after applying the patches my device is till hanging.

Re: [PATCH v3 3/6] spi: sun6i: restrict transfer length in PIO-mode

2018-04-05 Thread Sergey Suloev


On 04/05/2018 12:19 PM, Maxime Ripard wrote:

On Wed, Apr 04, 2018 at 02:35:14PM +0300, Sergey Suloev wrote:

On 04/04/2018 09:50 AM, Maxime Ripard wrote:

On Tue, Apr 03, 2018 at 06:44:46PM +0300, Sergey Suloev wrote:

There is no need to handle 3/4 empty interrupt as the maximum
supported transfer length in PIO mode is equal to FIFO depth,
i.e. 128 bytes for sun6i and 64 bytes for sun8i SoCs.

Changes in v3:
1) Restored processing of 3/4 FIFO full interrupt.

Signed-off-by: Sergey Suloev 
---
   drivers/spi/spi-sun6i.c | 41 +
   1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 78acc1f..c09ad10 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -207,7 +207,10 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool 
enable)
   static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
   {
-   return SUN6I_MAX_XFER_SIZE - 1;
+   struct spi_master *master = spi->master;
+   struct sun6i_spi *sspi = spi_master_get_devdata(master);
+
+   return sspi->fifo_depth;

Doesn't that effectively revert 3288d5cb40c0 ?

Why do you need to do so?

Need what?

Why do you need to revert that change.


Change is supposed to restrict max transfer size for PIO mode otherwise it
will fail.
The maximum transfer length = FIFO depth for PIO mode.

The point of that patch was precisely to allow to send more data than
the FIFO. You're breaking that behaviour without any justification,
and this is not ok.


I am sorry, but you can't. That's a hardware limitation.

   }
   static int sun6i_spi_prepare_message(struct spi_master *master,
@@ -255,8 +258,14 @@ static int sun6i_spi_transfer_one(struct spi_master 
*master,
int ret = 0;
u32 reg;
-   if (tfr->len > SUN6I_MAX_XFER_SIZE)
-   return -EINVAL;
+   /* A zero length transfer never finishes if programmed
+  in the hardware */

Improper comment style here. Please make sure to run checkpatch before
sending your patches.

ok

+   if (!tfr->len)
+   return 0;

Can that case even happen?

Not sure, just for safety.

+   /* Don't support transfer larger than the FIFO */
+   if (tfr->len > sspi->fifo_depth)
+   return -EMSGSIZE;

You're changing the return type, why?

As  a more appropriate one

The fact that it's more appropriate should be justified.


reinit_completion(&sspi->done);
sspi->tx_buf = tfr->tx_buf;
@@ -278,8 +287,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 */
trig_level = sspi->fifo_depth / 4 * 3;
sun6i_spi_write(sspi, SUN6I_FIFO_CTL_REG,
-   (trig_level << SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS) |
-   (trig_level << SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_BITS));
+   (trig_level << SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS));
reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
@@ -343,11 +351,8 @@ static int sun6i_spi_transfer_one(struct spi_master 
*master,
sun6i_spi_fill_fifo(sspi, sspi->fifo_depth);
/* Enable the interrupts */
-   sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
sun6i_spi_enable_interrupt(sspi, SUN6I_INT_CTL_TC |
 SUN6I_INT_CTL_RF_RDY);
-   if (tx_len > sspi->fifo_depth)
-   sun6i_spi_enable_interrupt(sspi, SUN6I_INT_CTL_TF_ERQ);

This would also need to be explained in your commit log.

What exactly and in what way ?

You should explain, at least:
A) What is the current behaviour
B) Why that is a problem, or what problem does it cause
C) What solution you implement and why you think it's justified


/* Start the transfer */
reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
@@ -376,7 +381,9 @@ out:
   static irqreturn_t sun6i_spi_handler(int irq, void *dev_id)
   {
struct sun6i_spi *sspi = dev_id;
-   u32 status = sun6i_spi_read(sspi, SUN6I_INT_STA_REG);
+   u32 status;
+
+   status = sun6i_spi_read(sspi, SUN6I_INT_STA_REG);

Why is this change needed?

A minor one, for readability.

That's arguable, and you should have a single logical change per
patch. So this doesn't belong in this one.

Maxime



___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/6] spi: sun6i: restrict transfer length in PIO-mode

2018-04-05 Thread Sergey Suloev


On 04/05/2018 04:17 PM, Mark Brown wrote:

On Thu, Apr 05, 2018 at 12:59:35PM +0300, Sergey Suloev wrote:

On 04/05/2018 12:19 PM, Maxime Ripard wrote:

The point of that patch was precisely to allow to send more data than
the FIFO. You're breaking that behaviour without any justification,
and this is not ok.

I am sorry, but you can't. That's a hardware limitation.

Are you positive about that?  Normally you can add things to hardware
FIFOs while they're being drained so so long as you can keep data
flowing in at least as fast as it's being consumed.


Well, normally yes, but this is not the case with the hardware that I 
own. My a20 (BPiM1+) and a31 (BPiM2) boards behaves differently. With a 
transfer larger than FIFO then TC interrupt never happens.

Re: [PATCH v3 3/6] spi: sun6i: restrict transfer length in PIO-mode

2018-04-06 Thread Sergey Suloev


On 04/06/2018 10:34 AM, Maxime Ripard wrote:

On Thu, Apr 05, 2018 at 04:44:16PM +0300, Sergey Suloev wrote:

On 04/05/2018 04:17 PM, Mark Brown wrote:

On Thu, Apr 05, 2018 at 12:59:35PM +0300, Sergey Suloev wrote:

On 04/05/2018 12:19 PM, Maxime Ripard wrote:

The point of that patch was precisely to allow to send more data than
the FIFO. You're breaking that behaviour without any justification,
and this is not ok.

I am sorry, but you can't. That's a hardware limitation.

Are you positive about that?  Normally you can add things to hardware
FIFOs while they're being drained so so long as you can keep data
flowing in at least as fast as it's being consumed.

Well, normally yes, but this is not the case with the hardware that I own.
My a20 (BPiM1+) and a31 (BPiM2) boards behaves differently. With a transfer
larger than FIFO then TC interrupt never happens.

Because you're not supposed to have a transfer larger than the FIFO,
but to have to setup at first a transfer the size of the FIFO, and
then when it's (or starts to be) depleted, fill it up again.


According to what you said the driver must implement 
"transfer_one_message" instead of "transfer_one"




That's the point of the patch you're reverting, and if it doesn't
work, you should make it work and not simply revert it.

Maxime

Re: [PATCH v3 3/6] spi: sun6i: restrict transfer length in PIO-mode

2018-04-06 Thread Sergey Suloev


On 04/06/2018 10:34 AM, Maxime Ripard wrote:

On Thu, Apr 05, 2018 at 04:44:16PM +0300, Sergey Suloev wrote:

On 04/05/2018 04:17 PM, Mark Brown wrote:

On Thu, Apr 05, 2018 at 12:59:35PM +0300, Sergey Suloev wrote:

On 04/05/2018 12:19 PM, Maxime Ripard wrote:

The point of that patch was precisely to allow to send more data than
the FIFO. You're breaking that behaviour without any justification,
and this is not ok.

I am sorry, but you can't. That's a hardware limitation.

Are you positive about that?  Normally you can add things to hardware
FIFOs while they're being drained so so long as you can keep data
flowing in at least as fast as it's being consumed.

Well, normally yes, but this is not the case with the hardware that I own.
My a20 (BPiM1+) and a31 (BPiM2) boards behaves differently. With a transfer
larger than FIFO then TC interrupt never happens.

Because you're not supposed to have a transfer larger than the FIFO,
but to have to setup at first a transfer the size of the FIFO, and
then when it's (or starts to be) depleted, fill it up again.
According to what you said the driver must implement 
"transfer_one_message" instead of "transfer_one",
because the maximum transfer length is 64 bytes (for sun4i) and we 
shouldn't allow "transfer_one" handle

more than 64 bytes. Otherwise it breaks the concept.


That's the point of the patch you're reverting, and if it doesn't
work, you should make it work and not simply revert it.

Maxime

69 matches

Mail list logo