Re: [PATCH v2 4/4] ARM: dts: tegra20: Revert "Fix ULPI regression on Tegra20"

[Dmitry Osipenko]

On 04.05.2018 13:40, Thierry Reding wrote:
> On Fri, May 04, 2018 at 01:55:37AM +0300, Dmitry Osipenko wrote:
>> Commit 4c9a27a6c66d ("ARM: tegra: Fix ULPI regression on Tegra20") changed
>> "ulpi-link" clock from CDEV2 to PLL_P_OUT4. Turned out that PLL_P_OUT4 is
>> the parent of CDEV2 clock and original clock setup of "ulpi-link" was
>> correct. The reverted patch was fixing USB for one board and broke the
>> other, now Tegra's clk driver correctly sets parent for the CDEV2 clock
>> and hence patch could be reverted safely, restoring USB for all of the
>> boards.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> Reviewed-by: Marcel Ziswiler <mar...@ziswiler.com>
>> Tested-by: Marcel Ziswiler <mar...@ziswiler.com>
>> Tested-by: Marc Dietrich <marvi...@gmx.de>
>> ---
>>  arch/arm/boot/dts/tegra20.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Is it still true that this patch alone fixes the known regression? I'm
> just asking because the remainder of the series, even though it sounds
> to be the right thing to do, is fairly big for a fix against v4.17.

Yes, Marcel says that reverting just this patch works for him now.

> So if this alone fixes the regression I think it'd be best to queue it
> up for v4.17 and get the rest of the patches into v4.18.

Sounds good.

Re: [PATCH v1 1/5] drm/tegra: dc: Balance IOMMU group refcounting

[Dmitry Osipenko]

On 04.05.2018 13:45, Thierry Reding wrote:
> On Fri, May 04, 2018 at 02:47:19AM +0300, Dmitry Osipenko wrote:
>> Remove unneeded iommu_group_get() and add missing iommu_group_put(),
>> correcting IOMMU group refcount. This is a minor correction / cleanup that
>> doesn't really fix anything because Tegra's IOMMU driver are built-in and
>> hence groups refcounting can't hold IOMMU driver from unloading.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>  drivers/gpu/drm/tegra/dc.c  | 35 +--
>>  drivers/gpu/drm/tegra/dc.h  |  2 +-
>>  drivers/gpu/drm/tegra/drm.h |  2 +-
>>  3 files changed, 19 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index d7a0f62c6e2e..9fc34c6a7218 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>> @@ -1927,7 +1927,6 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
>>  static int tegra_dc_init(struct host1x_client *client)
>>  {
>>  struct drm_device *drm = dev_get_drvdata(client->parent);
>> -struct iommu_group *group = iommu_group_get(client->dev);
>>  unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED;
>>  struct tegra_dc *dc = host1x_client_to_dc(client);
>>  struct tegra_drm *tegra = drm->dev_private;
>> @@ -1939,20 +1938,21 @@ static int tegra_dc_init(struct host1x_client 
>> *client)
>>  if (!dc->syncpt)
>>  dev_warn(dc->dev, "failed to allocate syncpoint\n");
>>  
>> -if (group && tegra->domain) {
>> -if (group != tegra->group) {
>> -err = iommu_attach_group(tegra->domain, group);
>> +if (tegra->domain) {
>> +dc->group = iommu_group_get(client->dev);
>> +
>> +if (dc->group && dc->group != tegra->dc_group) {
>> +err = iommu_attach_group(tegra->domain, dc->group);
>>  if (err < 0) {
>>  dev_err(dc->dev,
>>  "failed to attach to domain: %d\n",
>>  err);
>> +iommu_group_put(dc->group);
>>  return err;
>>  }
>>  
>> -tegra->group = group;
>> +tegra->dc_group = dc->group;
>>  }
>> -
>> -dc->domain = tegra->domain;
>>  }
>>  
>>  if (dc->soc->wgrps)
>> @@ -2017,13 +2017,13 @@ static int tegra_dc_init(struct host1x_client 
>> *client)
>>  if (!IS_ERR(primary))
>>  drm_plane_cleanup(primary);
>>  
>> -if (group && dc->domain) {
>> -if (group == tegra->group) {
>> -iommu_detach_group(dc->domain, group);
>> -tegra->group = NULL;
>> +if (dc->group) {
>> +if (dc->group && dc->group == tegra->dc_group) {
> 
> You can drop the dc->group check from the second conditional.

Indeed.

>> +iommu_detach_group(tegra->domain, dc->group);
>> +tegra->dc_group = NULL;
>>  }
>>  
>> -dc->domain = NULL;
>> +iommu_group_put(dc->group);
>>  }
>>  
>>  return err;
>> @@ -2032,7 +2032,6 @@ static int tegra_dc_init(struct host1x_client *client)
>>  static int tegra_dc_exit(struct host1x_client *client)
>>  {
>>  struct drm_device *drm = dev_get_drvdata(client->parent);
>> -struct iommu_group *group = iommu_group_get(client->dev);
>>  struct tegra_dc *dc = host1x_client_to_dc(client);
>>  struct tegra_drm *tegra = drm->dev_private;
>>  int err;
>> @@ -2045,13 +2044,13 @@ static int tegra_dc_exit(struct host1x_client 
>> *client)
>>  return err;
>>  }
>>  
>> -if (group && dc->domain) {
>> -if (group == tegra->group) {
>> -iommu_detach_group(dc->domain, group);
>> -tegra->group = NULL;
>> +if (dc->group) {
>> +if (dc->group && dc->group == tegra->dc_group) {
>> +iommu_detach_group(tegra->domain, dc->group);
>> +tegra->dc_group = NULL;
>>  }
>>  
>> 

Re: [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's

[Dmitry Osipenko]

On 04.05.2018 15:15, Thierry Reding wrote:
> On Fri, May 04, 2018 at 03:08:43AM +0300, Dmitry Osipenko wrote:
>> Older Tegra's do not support planes z position handling in hardware,
>> but HW provides knobs for zPos implementation in software.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>  drivers/gpu/drm/tegra/dc.c| 134 ---
>>  drivers/gpu/drm/tegra/plane.c | 193 --
>>  drivers/gpu/drm/tegra/plane.h |  13 ++-
>>  3 files changed, 244 insertions(+), 96 deletions(-)
> 
> This is obviously a lot to review, but it looks pretty good. On minor
> comment below.
> 
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index 2e81142281c3..ba5481cd470d 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>> @@ -162,29 +162,90 @@ static void tegra_plane_setup_blending_legacy(struct 
>> tegra_plane *plane)
> [...]
>> +case 1:
>> +/*
>> + * When both middle and topmost windows have an alpha,
>> + * these windows a mixed together and then the result
>> + * is blended over the bottom window.
>> + */
>> +if ((state->blending[0].alpha &&
>> + state->blending[0].top))
> 
> There seems to be one pair of parentheses too much here.

Good catch ;)

Re: [PATCH v2 0/4] Restore ULPI USB on Tegra20

[Dmitry Osipenko]

On 04.05.2018 16:33, Marcel Ziswiler wrote:
> Hi Dmitry
> 
> On Fri, 2018-05-04 at 01:55 +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> This series of patches fixes ULPI USB on Tegra20. The original
>> problem
>> was reported by Marcel Ziswiler, he found that "ulpi-link" clock was
>> incorrectly set to CDEV2 instead of PLL_P_OUT4. Marcel made a patch
>> that changed the "ulpi-link" clock to PLL_P_OUT4 and that fixed issue
>> with the USB for the devices that have CDEV2 being enabled by
>> bootloader.
>> The patch got into the kernel and later Marc Dietrich found that USB
>> stopped working on the "paz00" Tegra20 board. After a bit of
>> discussion
>> was revealed that PLL_P_OUT4 is the parent clock of the CDEV2 and
>> clock
>> driver was setting CDEV2's parent incorrectly. The parent clock is
>> actually
>> determined by the pinmuxing config of CDEV2 pingroup. This patchset
>> fixes
>> the parent of CDEV2 clock by making Tegra's pinctrl driver a clock
>> provider,
>> providing CDEV1/2 clock muxes (thanks to Peter De Schrijver for the
>> suggestion), and then setting these clock muxes as parents for the
>> CDEV1/2
>> clocks. In the end Marcel's CDEV2->PLL_P_OUT4 change is reverted
>> since CDEV2
>> (aka MCLK2) is the actual clock source for "ulpi-link".
>>
>> Changelog:
>>
>> v2:
>>  - Added new patch "Add quirk for getting CDEV1/2 clocks",
>> assuring
>>that clk user won't get CDEV1/2 clocks until parent clk muxes
>>are available, i.e. resolves potential issue with CDEV-user
>> driver
>>vs pinctrl driver probe order.
>>
>>  - Factored out "pinctrl" patch from the patchset as was
>> requested by
>>Linus Walleij.
>>
>>  - Addressed v1 review comments: fixed swapped DEV1/2 clk div
>> bits,
>>made DEV1/2 divs read-only, etc minor changes.
>>
>> Dmitry Osipenko (4):
>>   clk: tegra20: Add DEV1/DEV2 OSC dividers
>>   clk: tegra20: Correct parents of CDEV1/2 clocks
>>   clk: tegra: Add quirk for getting CDEV1/2 clocks
>>   ARM: dts: tegra20: Revert "Fix ULPI regression on Tegra20"
>>
>>  arch/arm/boot/dts/tegra20.dtsi  |  2 +-
>>  drivers/clk/tegra/clk-tegra20.c | 20 +++
>>  drivers/clk/tegra/clk.c | 34
>> -
>>  3 files changed, 50 insertions(+), 6 deletions(-)
> 
> Beautiful (;-p).
> 
> root@colibri-t20:~# cat /sys/kernel/debug/clk/clk_summary | grep -E
> 'pll_p_out4|cdev2'
>   pll_p_out4_div  1102400  
> 0 0  
>  pll_p_out4   2202400  
> 0 0  
> cdev2_mux 1102400  
> 0 0  
>cdev2  1102400  
> 0 0  
> 
> You may add my reviewed and tested bys to the remaining patches as
> well.
> 
> Thanks!
Awesome, thank you!

[PATCH v2 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's

[Dmitry Osipenko]

Older Tegra's do not support planes z position handling in hardware,
but HW provides knobs for zPos implementation in software.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/gpu/drm/tegra/dc.c| 134 ---
 drivers/gpu/drm/tegra/plane.c | 193 --
 drivers/gpu/drm/tegra/plane.h |  13 ++-
 3 files changed, 244 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 79aac38f4ed2..be624ac700a8 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -162,29 +162,90 @@ static void tegra_plane_setup_blending_legacy(struct 
tegra_plane *plane)
u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
 BLEND_COLOR_KEY_NONE;
u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+   u32 blending[2];
struct tegra_plane_state *state;
unsigned int i;
 
+   /* disable blending for non-overlapping case */
+   tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY);
+   tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
+
state = to_tegra_plane_state(plane->base.state);
 
-   /* alpha contribution is 1 minus sum of overlapping windows */
-   for (i = 0; i < 3; i++) {
-   if (state->dependent[i])
-   background[i] |= BLEND_CONTROL_DEPENDENT;
-   }
+   if (state->opaque) {
+   /*
+* Since custom fix-weight blending isn't utilized and weight
+* of top window is set to max, we can enforce dependent
+* blending which in this case results in transparent bottom
+* window if top window is opaque and if top window enables
+* alpha blending, then bottom window is getting alpha value
+* of 1 minus the sum of alpha components of the overlapping
+* plane.
+*/
+   background[0] |= BLEND_CONTROL_DEPENDENT;
+   background[1] |= BLEND_CONTROL_DEPENDENT;
 
-   /* enable alpha blending if pixel format has an alpha component */
-   if (!state->opaque)
+   /*
+* The region where three windows overlap is the intersection
+* of the two regions where two windows overlap. It contributes
+* to the area if all of the windows on top of it have an alpha
+* component.
+*/
+   switch (state->base.normalized_zpos) {
+   case 0:
+   if (state->blending[0].alpha &&
+   state->blending[1].alpha)
+   background[2] |= BLEND_CONTROL_DEPENDENT;
+   break;
+
+   case 1:
+   background[2] |= BLEND_CONTROL_DEPENDENT;
+   break;
+   }
+   } else {
+   /*
+* Enable alpha blending if pixel format has an alpha
+* component.
+*/
foreground |= BLEND_CONTROL_ALPHA;
 
-   /*
-* Disable blending and assume Window A is the bottom-most window,
-* Window C is the top-most window and Window B is in the middle.
-*/
-   tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY);
-   tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
+   /*
+* If any of the windows on top of this window is opaque, it
+* will completely conceal this window within that area. If
+* top window has an alpha component, it is blended over the
+* bottom window.
+*/
+   for (i = 0; i < 2; i++) {
+   if (state->blending[i].alpha &&
+   state->blending[i].top)
+   background[i] |= BLEND_CONTROL_DEPENDENT;
+   }
+
+   switch (state->base.normalized_zpos) {
+   case 0:
+   if (state->blending[0].alpha &&
+   state->blending[1].alpha)
+   background[2] |= BLEND_CONTROL_DEPENDENT;
+   break;
+
+   case 1:
+   /*
+* When both middle and topmost windows have an alpha,
+* these windows a mixed together and then the result
+* is blended over the bottom window.
+*/
+   if (state->blending[0].alpha &&
+   state->blending[0].top)
+   background[2] |= BLEND_CONTROL_ALPHA;
 
-   switch (plane->index) {
+   if (state->blending[1].alpha &&
+   

[PATCH v2 3/3] drm/tegra: dc: Rename supports_blending to has_legacy_blending

[Dmitry Osipenko]

Older Tegra's support blending. Rename SoC info entry supports_blending
to has_legacy_blending to eliminate confusion.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/gpu/drm/tegra/dc.c | 20 ++--
 drivers/gpu/drm/tegra/dc.h |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index be624ac700a8..c28a0a2d496b 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -508,10 +508,10 @@ static void tegra_dc_setup_window(struct tegra_plane 
*plane,
 
tegra_plane_writel(plane, value, DC_WIN_WIN_OPTIONS);
 
-   if (dc->soc->supports_blending)
-   tegra_plane_setup_blending(plane, window);
-   else
+   if (dc->soc->has_legacy_blending)
tegra_plane_setup_blending_legacy(plane);
+   else
+   tegra_plane_setup_blending(plane, window);
 }
 
 static const u32 tegra20_primary_formats[] = {
@@ -617,7 +617,7 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 * the corresponding opaque formats. However, the opaque formats can
 * be emulated by disabling alpha blending for the plane.
 */
-   if (!dc->soc->supports_blending) {
+   if (dc->soc->has_legacy_blending) {
err = tegra_plane_setup_legacy_state(tegra, plane_state);
if (err < 0)
return err;
@@ -2099,7 +2099,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info 
= {
.supports_interlacing = false,
.supports_cursor = false,
.supports_block_linear = false,
-   .supports_blending = false,
+   .has_legacy_blending = true,
.pitch_align = 8,
.has_powergate = false,
.coupled_pm = true,
@@ -2118,7 +2118,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info 
= {
.supports_interlacing = false,
.supports_cursor = false,
.supports_block_linear = false,
-   .supports_blending = false,
+   .has_legacy_blending = true,
.pitch_align = 8,
.has_powergate = false,
.coupled_pm = false,
@@ -2137,7 +2137,7 @@ static const struct tegra_dc_soc_info 
tegra114_dc_soc_info = {
.supports_interlacing = false,
.supports_cursor = false,
.supports_block_linear = false,
-   .supports_blending = false,
+   .has_legacy_blending = true,
.pitch_align = 64,
.has_powergate = true,
.coupled_pm = false,
@@ -2156,7 +2156,7 @@ static const struct tegra_dc_soc_info 
tegra124_dc_soc_info = {
.supports_interlacing = true,
.supports_cursor = true,
.supports_block_linear = true,
-   .supports_blending = true,
+   .has_legacy_blending = false,
.pitch_align = 64,
.has_powergate = true,
.coupled_pm = false,
@@ -2175,7 +2175,7 @@ static const struct tegra_dc_soc_info 
tegra210_dc_soc_info = {
.supports_interlacing = true,
.supports_cursor = true,
.supports_block_linear = true,
-   .supports_blending = true,
+   .has_legacy_blending = false,
.pitch_align = 64,
.has_powergate = true,
.coupled_pm = false,
@@ -2228,7 +2228,7 @@ static const struct tegra_dc_soc_info 
tegra186_dc_soc_info = {
.supports_interlacing = true,
.supports_cursor = true,
.supports_block_linear = true,
-   .supports_blending = true,
+   .has_legacy_blending = false,
.pitch_align = 64,
.has_powergate = false,
.coupled_pm = false,
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 8673cd97afc3..0774356c845a 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -55,7 +55,7 @@ struct tegra_dc_soc_info {
bool supports_interlacing;
bool supports_cursor;
bool supports_block_linear;
-   bool supports_blending;
+   bool has_legacy_blending;
unsigned int pitch_align;
bool has_powergate;
bool coupled_pm;
-- 
2.17.0

[PATCH v2 0/3] Couple more DRM plane features for Tegra

[Dmitry Osipenko]

Hi,

This series improves DRM plane support by supporting zPos on older Tegra's
and enabling plane scaling filters (up to Tegra210).

Changelog:

v2:
- Addressed v1 review comments.

Dmitry Osipenko (3):
  drm/tegra: dc: Enable plane scaling filters
  drm/tegra: plane: Implement zPos plane property for older Tegra's
  drm/tegra: dc: Rename supports_blending to has_legacy_blending

 drivers/gpu/drm/tegra/dc.c| 235 +++---
 drivers/gpu/drm/tegra/dc.h|   9 +-
 drivers/gpu/drm/tegra/plane.c | 193 
 drivers/gpu/drm/tegra/plane.h |  13 +-
 4 files changed, 343 insertions(+), 107 deletions(-)

-- 
2.17.0

[PATCH v2 1/3] drm/tegra: dc: Enable plane scaling filters

[Dmitry Osipenko]

Currently resized plane produces a "pixelated" image which doesn't look
nice, especially in a case of a video overlay. Enable scaling filters that
significantly improve image quality of a scaled overlay.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/gpu/drm/tegra/dc.c | 81 ++
 drivers/gpu/drm/tegra/dc.h |  7 
 2 files changed, 88 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 9f83a65b5ea9..79aac38f4ed2 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -224,6 +224,39 @@ static void tegra_plane_setup_blending(struct tegra_plane 
*plane,
tegra_plane_writel(plane, value, DC_WIN_BLEND_LAYER_CONTROL);
 }
 
+static bool tegra_dc_window_can_use_horizontal_filtering(
+   struct tegra_plane *plane,
+   const struct tegra_dc_window *window)
+{
+   struct tegra_dc *dc = plane->dc;
+
+   if (window->src.w == window->dst.w)
+   return false;
+
+   if (plane->index == 0 && dc->soc->has_win_a_without_filters)
+   return false;
+
+   return true;
+}
+
+static bool tegra_dc_window_can_use_vertical_filtering(
+   struct tegra_plane *plane,
+   const struct tegra_dc_window *window)
+{
+   struct tegra_dc *dc = plane->dc;
+
+   if (window->src.h == window->dst.h)
+   return false;
+
+   if (plane->index == 0 && dc->soc->has_win_a_without_filters)
+   return false;
+
+   if (plane->index == 2 && dc->soc->has_win_c_without_vert_filter)
+   return false;
+
+   return true;
+}
+
 static void tegra_dc_setup_window(struct tegra_plane *plane,
  const struct tegra_dc_window *window)
 {
@@ -361,6 +394,44 @@ static void tegra_dc_setup_window(struct tegra_plane 
*plane,
if (window->bottom_up)
value |= V_DIRECTION;
 
+   if (tegra_dc_window_can_use_horizontal_filtering(plane, window)) {
+   /*
+* Enable horizontal 6-tap filter and set filtering
+* coefficients to the default values defined in TRM.
+*/
+   tegra_plane_writel(plane, 0x8000, DC_WIN_H_FILTER_P(0));
+   tegra_plane_writel(plane, 0x3e087ce1, DC_WIN_H_FILTER_P(1));
+   tegra_plane_writel(plane, 0x3b117ac1, DC_WIN_H_FILTER_P(2));
+   tegra_plane_writel(plane, 0x591b73aa, DC_WIN_H_FILTER_P(3));
+   tegra_plane_writel(plane, 0x57256d9a, DC_WIN_H_FILTER_P(4));
+   tegra_plane_writel(plane, 0x552f668b, DC_WIN_H_FILTER_P(5));
+   tegra_plane_writel(plane, 0x73385e8b, DC_WIN_H_FILTER_P(6));
+   tegra_plane_writel(plane, 0x72435583, DC_WIN_H_FILTER_P(7));
+   tegra_plane_writel(plane, 0x714c4c8b, DC_WIN_H_FILTER_P(8));
+   tegra_plane_writel(plane, 0x70554393, DC_WIN_H_FILTER_P(9));
+   tegra_plane_writel(plane, 0x715e389b, DC_WIN_H_FILTER_P(10));
+   tegra_plane_writel(plane, 0x71662faa, DC_WIN_H_FILTER_P(11));
+   tegra_plane_writel(plane, 0x536d25ba, DC_WIN_H_FILTER_P(12));
+   tegra_plane_writel(plane, 0x55731bca, DC_WIN_H_FILTER_P(13));
+   tegra_plane_writel(plane, 0x387a11d9, DC_WIN_H_FILTER_P(14));
+   tegra_plane_writel(plane, 0x3c7c08f1, DC_WIN_H_FILTER_P(15));
+
+   value |= H_FILTER;
+   }
+
+   if (tegra_dc_window_can_use_vertical_filtering(plane, window)) {
+   unsigned int i, k;
+
+   /*
+* Enable vertical 2-tap filter and set filtering
+* coefficients to the default values defined in TRM.
+*/
+   for (i = 0, k = 128; i < 16; i++, k -= 8)
+   tegra_plane_writel(plane, k, DC_WIN_V_FILTER_P(i));
+
+   value |= V_FILTER;
+   }
+
tegra_plane_writel(plane, value, DC_WIN_WIN_OPTIONS);
 
if (dc->soc->supports_blending)
@@ -1978,6 +2049,8 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info 
= {
.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
.overlay_formats = tegra20_overlay_formats,
.modifiers = tegra20_modifiers,
+   .has_win_a_without_filters = true,
+   .has_win_c_without_vert_filter = true,
 };
 
 static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
@@ -1995,6 +2068,8 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info 
= {
.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
.overlay_formats = tegra20_overlay_formats,
.modifiers = tegra20_modifiers,
+   .has_win_a_without_filters = false,
+   .has_win_c_without_vert_filter = false,
 

Re: [PATCH v1 05/11] cpufreq: tegra20: Clean up included headers

[Dmitry Osipenko]

On 18.05.2018 04:58, Viresh Kumar wrote:
> On 17-05-18, 21:00, Dmitry Osipenko wrote:
>> Remove unused/unneeded headers and sort them in the alphabet order.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>  drivers/cpufreq/tegra20-cpufreq.c | 11 ++-
>>  1 file changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>> b/drivers/cpufreq/tegra20-cpufreq.c
>> index 69f033d297e1..61f00d1cba26 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -16,16 +16,9 @@
>>   *
>>   */
>>  
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>>  #include 
>> -#include 
>> +#include 
>> +#include 
> 
> Ideally you should keep all the headers whose declarations your code
> is using directly. It may happen that removing above still compiles
> because cpufreq.h has included the headers indirectly for you. But
> that will break the day cpufreq.h doesn't need those headers anymore.
> 
> So just make sure you aren't using any of them in your code. For
> example you are using bool in your code and so you shouldn't remove
> types.h ? Same for init.h as you are using __init and __exit.

The preference on includes seems to vary among maintainers. I've seen other
opinion that encouraged to minimize included headers and only add the headers
when compilation breaks.

I'll revisit this patch and keep init.h and others in v2 since you prefer that
way. Thank you for the review.

Re: [PATCH v1 10/11] cpufreq: tegra20: Wrap cpufreq into platform driver

[Dmitry Osipenko]

On 18.05.2018 05:07, Viresh Kumar wrote:
> On 17-05-18, 21:00, Dmitry Osipenko wrote:
>> -static int __init tegra_cpufreq_init(void)
>> +static int tegra20_cpufreq_probe(struct platform_device *pdev)
>>  {
>> +struct tegra20_cpufreq_data *data;
>>  int err;
>>  
>> -if (!of_machine_is_compatible("nvidia,tegra20"))
>> -return -ENODEV;
> 
> So this stuff wasn't really required as you are getting rid of that in
> the same series. Should we really add it then ? Maybe ..
> 

It's not strictly needed, but I'd prefer to keep that stuff for clarity as it
kinda shows the way that led to the final result.

[snip]

Re: [PATCH v1 10/11] cpufreq: tegra20: Wrap cpufreq into platform driver

[Dmitry Osipenko]

On 18.05.2018 12:07, Thierry Reding wrote:
> On Thu, May 17, 2018 at 09:00:55PM +0300, Dmitry Osipenko wrote:
>> Currently tegra20-cpufreq kernel module isn't getting autoloaded because
>> there is no device associated with the module, this is one of two patches
>> that resolves the module autoloading. This patch adds a module alias that
>> will associate the tegra20-cpufreq kernel module with the platform device,
>> other patch will instantiate the actual platform device. And now it makes
>> sense to wrap cpufreq driver into a platform driver for consistency.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>  drivers/cpufreq/tegra20-cpufreq.c | 116 +++---
>>  1 file changed, 73 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>> b/drivers/cpufreq/tegra20-cpufreq.c
>> index c0a7b5a78aa6..f9d02a28df9e 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -19,7 +19,7 @@
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>> +#include 
>>  
>>  static struct cpufreq_frequency_table freq_table[] = {
>>  { .frequency = 216000 },
>> @@ -33,15 +33,19 @@ static struct cpufreq_frequency_table freq_table[] = {
>>  { .frequency = CPUFREQ_TABLE_END },
>>  };
>>  
>> -static struct clk *cpu_clk;
>> -static struct clk *pll_x_clk;
>> -static struct clk *pll_p_clk;
>> -static bool pll_x_prepared;
>> +struct tegra20_cpufreq_data {
> 
> Nit: I'm not a big fan of _data suffixes because they are completely
> redundant. Any data structure by definition hosts data, so I'd just drop
> that.

Okay, I'll drop it in v2.

> [...]
>> @@ -152,55 +161,76 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
>>  .suspend= cpufreq_generic_suspend,
>>  };
>>  
>> -static int __init tegra_cpufreq_init(void)
>> +static int tegra20_cpufreq_probe(struct platform_device *pdev)
>>  {
>> +struct tegra20_cpufreq_data *data;
>>  int err;
>>  
>> -if (!of_machine_is_compatible("nvidia,tegra20"))
>> -return -ENODEV;
>> +data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
>> +if (!data)
>> +return -ENOMEM;
>>  
>> -cpu_clk = clk_get_sys(NULL, "cclk");
>> -if (IS_ERR(cpu_clk))
>> -return PTR_ERR(cpu_clk);
>> +data->cpu_clk = clk_get_sys(NULL, "cclk");
>> +if (IS_ERR(data->cpu_clk))
>> +return PTR_ERR(data->cpu_clk);
>>  
>> -pll_x_clk = clk_get_sys(NULL, "pll_x");
>> -if (IS_ERR(pll_x_clk)) {
>> -err = PTR_ERR(pll_x_clk);
>> +data->pll_x_clk = clk_get_sys(NULL, "pll_x");
>> +if (IS_ERR(data->pll_x_clk)) {
>> +err = PTR_ERR(data->pll_x_clk);
>>  goto put_cpu;
>>  }
>>  
>> -pll_p_clk = clk_get_sys(NULL, "pll_p");
>> -if (IS_ERR(pll_p_clk)) {
>> -err = PTR_ERR(pll_p_clk);
>> +data->pll_p_clk = clk_get_sys(NULL, "pll_p");
>> +if (IS_ERR(data->pll_p_clk)) {
>> +err = PTR_ERR(data->pll_p_clk);
>>  goto put_pll_x;
>>  }
>>  
>> +data->dev = >dev;
>> +
>> +tegra_cpufreq_driver.driver_data = data;
> 
> Couldn't this be embedded into struct tegra20_cpufreq_data? Moving
> everything but this into a per-device data structure seems half-baked.

That's a good suggestions, thank you.

Re: [PATCH v1 11/11] ARM: tegra: Create platform device for tegra20-cpufreq driver

[Dmitry Osipenko]

On 18.05.2018 12:13, Thierry Reding wrote:
> On Thu, May 17, 2018 at 09:00:56PM +0300, Dmitry Osipenko wrote:
>> Tegra20-cpufreq driver require a platform device in order to be loaded,
>> instantiate a simple platform device for the driver during of the machines
>> late initialization.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>  arch/arm/mach-tegra/tegra.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>> index 02e712d2ea30..f9587be48235 100644
>> --- a/arch/arm/mach-tegra/tegra.c
>> +++ b/arch/arm/mach-tegra/tegra.c
>> @@ -97,6 +97,10 @@ static void __init tegra_dt_init_late(void)
>>  if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
>>  of_machine_is_compatible("compal,paz00"))
>>  tegra_paz00_wifikill_init();
>> +
>> +if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
>> +of_machine_is_compatible("nvidia,tegra20"))
>> +platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
>>  }
>>  
>>  static const char * const tegra_dt_board_compat[] = {
> 
> Tegra124 has a CPU frequency driver that is similar to this and it
> contains code that will instantiate the platform device from the CPU
> frequency driver's module_init function.
> 
> I think the primary reason for doing that was to not tie the code to
> 32-bit ARM, even though it never runs on anything but that, so it's
> slightly over-engineered.

The tegra124-cpufreq driver likely to be broken in regards to the module loading
as platform_device_register_simple() seem to be allowed invoked only from the
kernel itself.

Having platform_device_register_simple in the kernels module leads to a such
errors on the module loading:

# section 5 reloc 7 sym 'memset': relocation 10 out of range (0xbf805030 ->
0xc088c481)

Initially I wanted to make tegra124-cpufreq a module_platform_driver, but
decided to postpone that, as the driver is shared with ARM64 Tegra132 which
doesn't have machine-init code. Maybe we could move the devices instantiation to
something common like driver/soc/tegra/devices.c later.

> I don't mind either way, and it's easy enough to change this to
> something else later on if we want. I'll pick this up into the Tegra
> tree.
Okay, thanks.

Re: [PATCH v1 00/11] Clean up Tegra20 cpufreq driver

[Dmitry Osipenko]

On 18.05.2018 10:30, Rafael J. Wysocki wrote:
> On Thursday, May 17, 2018 8:00:45 PM CEST Dmitry Osipenko wrote:
>> Hello,
>>
>> Recently Peter Geis (who is working on Tegra30 cpufreq driver) asked me how
>> tegra20-cpufreq driver is getting loaded. After taking a look at the code
>> it became apparent that the drivers code has been rusted a tad and so this
>> series is intended to refresh the drivers code by disallowing module to be
>> loaded on non-Tegra20 machines, by cleaning whitespaces in the code, removing
>> dead EMC code and in the end by allowing tegra20-cpufreq to be built as a
>> loadable module.
>>
>> Please review, thanks.
>>
>> Dmitry Osipenko (11):
>>   cpufreq: tegra20: Change module description
>>   cpufreq: tegra20: Clean up whitespaces in the code
>>   cpufreq: tegra20: Remove EMC clock usage
>>   cpufreq: tegra20: Release clocks properly
>>   cpufreq: tegra20: Clean up included headers
>>   cpufreq: tegra20: Remove unneeded check in tegra_cpu_init
>>   cpufreq: tegra20: Check if this is Tegra20 machine
>>   cpufreq: tegra20: Remove unneeded variable initialization
>>   cpufreq: tegra20: Allow cpufreq driver to be built as loadable module
>>   cpufreq: tegra20: Wrap cpufreq into platform driver
>>   ARM: tegra: Create platform device for tegra20-cpufreq driver
>>
>>  arch/arm/mach-tegra/tegra.c   |   4 +
>>  drivers/cpufreq/Kconfig.arm   |   2 +-
>>  drivers/cpufreq/tegra20-cpufreq.c | 172 +-
>>  3 files changed, 101 insertions(+), 77 deletions(-)
>>
>>
> 
> It looks like Viresh has ACKed the majority of the patches in this series,
> but there are a few where he had comments.
> 
> Please fix up these and resend the entire series with the Acked-by tags from
> Viresh added where applicable.

Sure, I'll address the Viresh's review comments in the v2 that I'll probably
prepare and send out later today. I'll also add another minor-cleanup patch to
the series that removes unnecessary brackets in the code.

Thierry / Jon, please let me if there is anything you disagree-with in the
series as I'd like to avoid re-sending multiple times.

Thanks.

Re: [PATCH v3] pinctrl: tegra20: Provide CDEV1/2 clock muxes

[Dmitry Osipenko]

On 18.05.2018 13:39, Thierry Reding wrote:
> On Tue, May 08, 2018 at 07:26:04PM +0300, Dmitry Osipenko wrote:
>> Muxing of pins MCLK1/2 determine the muxing of the corresponding clocks.
>> Make pinctrl driver to provide clock muxes for the CDEV1/2 pingroups, so
>> that main clk-controller driver could get an actual parent clock for the
>> CDEV1/2 clocks.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> Reviewed-by: Marcel Ziswiler <mar...@ziswiler.com>
>> Tested-by: Marcel Ziswiler <mar...@ziswiler.com>
>> Tested-by: Marc Dietrich <marvi...@gmx.de>
>> Acked-by: Peter De Schrijver <pdeschrij...@nvidia.com>
>> ---
>>
>> Changelog:
>>
>> v3:
>>  - No change.
>>
>> v2:
>>  - See changelog of "Restore ULPI USB on Tegra20" v2 series.
>>
>>  drivers/pinctrl/tegra/pinctrl-tegra.c   | 11 -
>>  drivers/pinctrl/tegra/pinctrl-tegra.h   | 11 +
>>  drivers/pinctrl/tegra/pinctrl-tegra20.c | 30 -
>>  3 files changed, 40 insertions(+), 12 deletions(-)
> 
> Linus,
> 
> were you going to pick this up for v4.18? There's a runtime dependency
> on this from some clock patches that I'd like to get into v4.18.
> 
> I see I never gave my Acked-by on this particular patch, so here goes:
> 
> Acked-by: Thierry Reding <tred...@nvidia.com>

Linus applied the v2 of the patch [0]. Thanks for caring about it.

[0] https://www.spinics.net/lists/linux-tegra/msg1.html

Re: [PATCH v2 3/6] ARM: trusted_foundations: do not use naked function

[Dmitry Osipenko]

On 16.04.2018 21:21, Stefan Agner wrote:
> On 16.04.2018 18:08, Stephen Warren wrote:
>> On 04/16/2018 09:56 AM, Stefan Agner wrote:
>>> On 27.03.2018 14:16, Dmitry Osipenko wrote:
>>>> On 27.03.2018 14:54, Robin Murphy wrote:
>>>>> On 26/03/18 22:20, Dmitry Osipenko wrote:
>>>>>> On 25.03.2018 21:09, Stefan Agner wrote:
>>>>>>> As documented in GCC naked functions should only use Basic asm
>>>>>>> syntax. The Extended asm or mixture of Basic asm and "C" code is
>>>>>>> not guaranteed. Currently this works because it was hard coded
>>>>>>> to follow and check GCC behavior for arguments and register
>>>>>>> placement.
>>>>>>>
>>>>>>> Furthermore with clang using parameters in Extended asm in a
>>>>>>> naked function is not supported:
>>>>>>>     arch/arm/firmware/trusted_foundations.c:47:10: error: parameter
>>>>>>>     references not allowed in naked functions
>>>>>>>   : "r" (type), "r" (arg1), "r" (arg2)
>>>>>>>      ^
>>>>>>>
>>>>>>> Use a regular function to be more portable. This aligns also with
>>>>>>> the other smc call implementations e.g. in qcom_scm-32.c and
>>>>>>> bcm_kona_smc.c.
>>>>>>>
>>>>>>> Cc: Dmitry Osipenko <dig...@gmail.com>
>>>>>>> Cc: Stephen Warren <swar...@nvidia.com>
>>>>>>> Cc: Thierry Reding <tred...@nvidia.com>
>>>>>>> Signed-off-by: Stefan Agner <ste...@agner.ch>
>>>>>>> ---
>>>>>>> Changes in v2:
>>>>>>> - Keep stmfd/ldmfd to avoid potential ABI issues
>>>>>>>
>>>>>>>    arch/arm/firmware/trusted_foundations.c | 14 +-
>>>>>>>    1 file changed, 9 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/firmware/trusted_foundations.c
>>>>>>> b/arch/arm/firmware/trusted_foundations.c
>>>>>>> index 3fb1b5a1dce9..689e6565abfc 100644
>>>>>>> --- a/arch/arm/firmware/trusted_foundations.c
>>>>>>> +++ b/arch/arm/firmware/trusted_foundations.c
>>>>>>> @@ -31,21 +31,25 @@
>>>>>>>      static unsigned long cpu_boot_addr;
>>>>>>>    -static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
>>>>>>> +static void tf_generic_smc(u32 type, u32 arg1, u32 arg2)
>>>>>>>    {
>>>>>>> +    register u32 r0 asm("r0") = type;
>>>>>>> +    register u32 r1 asm("r1") = arg1;
>>>>>>> +    register u32 r2 asm("r2") = arg2;
>>>>>>> +
>>>>>>>    asm volatile(
>>>>>>>    ".arch_extension    sec\n\t"
>>>>>>> -    "stmfd    sp!, {r4 - r11, lr}\n\t"
>>>>>>> +    "stmfd    sp!, {r4 - r11}\n\t"
>>>>>>>    __asmeq("%0", "r0")
>>>>>>>    __asmeq("%1", "r1")
>>>>>>>    __asmeq("%2", "r2")
>>>>>>>    "mov    r3, #0\n\t"
>>>>>>>    "mov    r4, #0\n\t"
>>>>>>>    "smc    #0\n\t"
>>>>>>> -    "ldmfd    sp!, {r4 - r11, pc}"
>>>>>>> +    "ldmfd    sp!, {r4 - r11}\n\t"
>>>>>>>    :
>>>>>>> -    : "r" (type), "r" (arg1), "r" (arg2)
>>>>>>> -    : "memory");
>>>>>>> +    : "r" (r0), "r" (r1), "r" (r2)
>>>>>>> +    : "memory", "r3", "r12", "lr");
>>>>>>
>>>>>> Although seems "lr" won't be affected by SMC invocation because it 
>>>>>> should be
>>>>>> banked and hence could be omitted entirely from the code. Maybe somebody 
>>>>>> could
>>>>>> confirm this.
>>>>> Strictly per the letter of the architecture, the SMC could be trapped to 
>>>>> Hyp
>>>>> mode, and a hypervisor might clobber LR_usr in the process of forwarding 
>>>>> the
>>>>> call to the firmware secure monitor (since Hyp doesn't have a banked LR 
>>>>> of its
>>>>> own). Admittedly there are probably no real systems with the appropriate
>>>>> hardware/software combination to hit that, but on the other hand if this 
>>>>> gets
>>>>> inlined where the compiler has already created a stack frame then an LR 
>>>>> clobber
>>>>> is essentially free, so I reckon we're better off keeping it for 
>>>>> reassurance.
>>>>> This isn't exactly a critical fast path anyway.
>>>>
>>>> Okay, thank you for the clarification.
>>>
>>> So it seems this change is fine?
>>>
>>> Stephen, you picked up changes for this driver before, is this patch
>>> going through your tree?
>>
>> You had best ask Thierry; he's taken over Tegra maintenance upstream.
>> But that said, don't files in arch/arm go through Russell?
> 
> I think the last patches applied to that file went through your tree.
> 
> Thierry, Russel, any preferences?

I've been preparing patches for upstream to add initial support of L2 cache
maintance to TF / Tegra30 and noticed that without this patch I'm getting a hang
early in boot. That is because before this patch registers store / restore was
incorrect, probably the premature return (lr -> pc) causes stack corruption. Not
sure whether it's worth to backport this patch, but I want to see it at least in
-next.

Thierry, please take care of this patch. Thanks.

[PATCH v1 0/5] Initial support of Trusted Foundations on Tegra30

[Dmitry Osipenko]

Hello,

This series of patches brings initial support of Trusted Foundations to
Tegra30, that is to the consumer-grade Tegra30 devices which do not allow
to easily replace the proprietary bootloader. Support is initial because
this series implements only a proper CPU boot-up (main + secondary cores)
and a basic L2 cache maintenance that is done using the TF firmware.
Suspend-resume support is missing yet as I couldn't get it to work
(CPU hangs on resume from suspend after awhile and seems that is related
to inappropriately done cache maintenance during of suspend-resume using
the firmware), it is work-in-progress for now.

This patchset is partially based on the work done by Michał Mirosław [0].

Please review, thanks.

[0] https://www.spinics.net/lists/linux-tegra/msg30368.html

Dmitry Osipenko (5):
  ARM: trusted_foundations: Implement L2 cache initialization callback
  ARM: trusted_foundations: Provide information about whether firmware
is registered
  ARM: tegra: Setup L2 cache using Trusted Foundations firmware
  ARM: tegra: Don't apply CPU erratas in insecure mode
  ARM: tegra: Always boot CPU in ARM-mode

 arch/arm/firmware/trusted_foundations.c| 28 ++
 arch/arm/include/asm/trusted_foundations.h |  7 ++
 arch/arm/mach-tegra/reset-handler.S| 28 +++---
 arch/arm/mach-tegra/reset.c|  5 +++-
 arch/arm/mach-tegra/reset.h|  4 +++-
 arch/arm/mach-tegra/tegra.c| 15 
 6 files changed, 77 insertions(+), 10 deletions(-)

-- 
2.17.0

[PATCH v1 2/2] ARM: dts: tegra30: Add Memory Client reset to VDE

[Dmitry Osipenko]

Hook up Memory Client reset of the Video Decoder to the decoders DT node.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/boot/dts/tegra30.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 09087b9c5e26..3300ff976053 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -404,7 +404,8 @@
 ; /* SXE interrupt 
*/
interrupt-names = "sync-token", "bsev", "sxe";
clocks = <_car TEGRA30_CLK_VDE>;
-   resets = <_car 61>;
+   reset-names = "vde", "mc";
+   resets = <_car 61>, < TEGRA30_MC_RESET_VDE>;
};
 
apbmisc@7800 {
@@ -712,6 +713,7 @@
interrupts = ;
 
#iommu-cells = <1>;
+   #reset-cells = <1>;
};
 
fuse@7000f800 {
-- 
2.17.0

[PATCH v1 1/2] ARM: dts: tegra20: Add Memory Client reset to VDE

[Dmitry Osipenko]

Hook up Memory Client reset of the Video Decoder to the decoders DT node.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/boot/dts/tegra20.dtsi | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 983dd5c14794..f9495f12e731 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -282,7 +283,8 @@
 ; /* SXE interrupt 
*/
interrupt-names = "sync-token", "bsev", "sxe";
clocks = <_car TEGRA20_CLK_VDE>;
-   resets = <_car 61>;
+   reset-names = "vde", "mc";
+   resets = <_car 61>, < TEGRA20_MC_RESET_VDE>;
};
 
apbmisc@7800 {
@@ -593,11 +595,12 @@
clock-names = "pclk", "clk32k_in";
};
 
-   memory-controller@7000f000 {
+   mc: memory-controller@7000f000 {
compatible = "nvidia,tegra20-mc";
reg = <0x7000f000 0x024
   0x7000f03c 0x3c4>;
interrupts = ;
+   #reset-cells = <1>;
};
 
iommu@7000f024 {
-- 
2.17.0

[PATCH v1 5/5] ARM: tegra: Always boot CPU in ARM-mode

[Dmitry Osipenko]

CPU always jumps into the reset handler in ARM-mode from the Trusted
Foundations firmware, hence make CPU to always jump into kernel in
ARM-mode regardless of the firmware presence to support Thumb2 kernel + TF
case.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/mach-tegra/reset-handler.S | 1 +
 arch/arm/mach-tegra/reset.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-tegra/reset-handler.S 
b/arch/arm/mach-tegra/reset-handler.S
index d84c74a95806..7e15c3bdf118 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -115,6 +115,7 @@ ENTRY(__tegra_cpu_reset_handler_start)
  *   must be position-independent.
  */
 
+   .arm
.align L1_CACHE_SHIFT
 ENTRY(__tegra_cpu_reset_handler)
 
diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index b02ae7699842..3f1ef4561298 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -97,7 +97,7 @@ void __init tegra_cpu_reset_handler_init(void)
__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
*((u32 *)cpu_possible_mask);
__tegra_cpu_reset_handler_data[TEGRA_RESET_STARTUP_SECONDARY] =
-   __pa_symbol((void *)secondary_startup);
+   __pa_symbol((void *)secondary_startup_arm);
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.17.0

[PATCH v1 4/5] ARM: tegra: Don't apply CPU erratas in insecure mode

[Dmitry Osipenko]

CPU isn't allowed to touch secure registers while running under secure
monitor. Hence skip applying CPU erratas in the reset handler if Trusted
Foundations firmware presents.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/mach-tegra/reset-handler.S | 27 +++
 arch/arm/mach-tegra/reset.c |  3 +++
 arch/arm/mach-tegra/reset.h |  4 +++-
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-tegra/reset-handler.S 
b/arch/arm/mach-tegra/reset-handler.S
index 805f306fa6f7..d84c74a95806 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -121,6 +121,12 @@ ENTRY(__tegra_cpu_reset_handler)
cpsid   aif, 0x13   @ SVC mode, interrupts disabled
 
tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
+
+   adr r12, __tegra_cpu_reset_handler_data
+   ldr r0, [r12, #RESET_DATA(TF_PRESENT)]
+   cmp r0, #0
+   bne after_errata
+
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 t20_check:
cmp r6, #TEGRA20
@@ -155,7 +161,6 @@ after_errata:
and r10, r10, #0x3  @ R10 = CPU number
mov r11, #1
mov r11, r11, lsl r10   @ R11 = CPU mask
-   adr r12, __tegra_cpu_reset_handler_data
 
 #ifdef CONFIG_SMP
/* Does the OS know about this CPU? */
@@ -169,10 +174,9 @@ after_errata:
cmp r6, #TEGRA20
bne 1f
/* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
-   mov32   r5, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
mov r0, #CPU_NOT_RESETTABLE
cmp r10, #0
-   strneb  r0, [r5, #__tegra20_cpu1_resettable_status_offset]
+   strneb  r0, [r12, #RESET_DATA(RESETTABLE_STATUS)]
 1:
 #endif
 
@@ -278,13 +282,20 @@ ENDPROC(__tegra_cpu_reset_handler)
.type   __tegra_cpu_reset_handler_data, %object
.globl  __tegra_cpu_reset_handler_data
 __tegra_cpu_reset_handler_data:
-   .rept   TEGRA_RESET_DATA_SIZE
-   .long   0
-   .endr
+   .long   0   /* TEGRA_RESET_MASK_PRESENT */
+   .long   0   /* TEGRA_RESET_MASK_LP1 */
+   .long   0   /* TEGRA_RESET_MASK_LP2 */
+   .long   0   /* TEGRA_RESET_STARTUP_SECONDARY */
+   .long   0   /* TEGRA_RESET_STARTUP_LP2 */
+   .long   0   /* TEGRA_RESET_STARTUP_LP1 */
+
.globl  __tegra20_cpu1_resettable_status_offset
.equ__tegra20_cpu1_resettable_status_offset, \
. - __tegra_cpu_reset_handler_start
-   .byte   0
-   .align L1_CACHE_SHIFT
+   .long   0   /* TEGRA_RESET_RESETTABLE_STATUS */
 
+   .globl  __tegra_tf_present
+   .equ__tegra_tf_present, . - __tegra_cpu_reset_handler_start
+   .long   0   /* TEGRA_RESET_TF_PRESENT */
+   .align L1_CACHE_SHIFT
 ENTRY(__tegra_cpu_reset_handler_end)
diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index dc558892753c..b02ae7699842 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "iomap.h"
 #include "irammap.h"
@@ -89,6 +90,8 @@ static void __init tegra_cpu_reset_handler_enable(void)
 
 void __init tegra_cpu_reset_handler_init(void)
 {
+   __tegra_cpu_reset_handler_data[TEGRA_RESET_TF_PRESENT] =
+   trusted_foundations_registered();
 
 #ifdef CONFIG_SMP
__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index 9c479c7925b8..0d9ddc022ece 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -25,7 +25,9 @@
 #define TEGRA_RESET_STARTUP_SECONDARY  3
 #define TEGRA_RESET_STARTUP_LP24
 #define TEGRA_RESET_STARTUP_LP15
-#define TEGRA_RESET_DATA_SIZE  6
+#define TEGRA_RESET_RESETTABLE_STATUS  6
+#define TEGRA_RESET_TF_PRESENT 7
+#define TEGRA_RESET_DATA_SIZE  8
 
 #ifndef __ASSEMBLY__
 
-- 
2.17.0

[PATCH v1 2/5] ARM: trusted_foundations: Provide information about whether firmware is registered

[Dmitry Osipenko]

Add a helper that provides information about whether Trusted Foundations
firmware operations have been registered.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/firmware/trusted_foundations.c| 5 +
 arch/arm/include/asm/trusted_foundations.h | 7 +++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/firmware/trusted_foundations.c 
b/arch/arm/firmware/trusted_foundations.c
index 198ce5c75ca0..0428351574de 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -120,3 +120,8 @@ void of_register_trusted_foundations(void)
panic("Trusted Foundation: missing version-minor property\n");
register_trusted_foundations();
 }
+
+bool trusted_foundations_registered(void)
+{
+   return firmware_ops == _foundations_ops;
+}
diff --git a/arch/arm/include/asm/trusted_foundations.h 
b/arch/arm/include/asm/trusted_foundations.h
index 00748350cf72..bfd0d780824b 100644
--- a/arch/arm/include/asm/trusted_foundations.h
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct trusted_foundations_platform_data {
unsigned int version_major;
@@ -41,6 +42,7 @@ struct trusted_foundations_platform_data {
 
 void register_trusted_foundations(struct trusted_foundations_platform_data 
*pd);
 void of_register_trusted_foundations(void);
+bool trusted_foundations_registered(void);
 
 #else /* CONFIG_TRUSTED_FOUNDATIONS */
 
@@ -68,6 +70,11 @@ static inline void of_register_trusted_foundations(void)
if (of_find_compatible_node(NULL, NULL, "tlm,trusted-foundations"))
register_trusted_foundations(NULL);
 }
+
+static inline bool trusted_foundations_registered(void)
+{
+   return false;
+}
 #endif /* CONFIG_TRUSTED_FOUNDATIONS */
 
 #endif
-- 
2.17.0

[PATCH v1 3/5] ARM: tegra: Setup L2 cache using Trusted Foundations firmware

[Dmitry Osipenko]

On Tegra20/30 L2 cache must be initialized using firmware call if CPU
is running in insecure mode. Initialize L2 cache and setup the outer-cache
callbacks in early boot using the firmware API.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/mach-tegra/tegra.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index f9587be48235..590b1cf1a8c4 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -70,9 +71,23 @@ u32 tegra_uart_config[3] = {
0,
 };
 
+static void __init tegra_trusted_foundations_l2x0_cache_init(void)
+{
+   if (IS_ENABLED(CONFIG_CACHE_L2X0) &&
+   IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
+   of_machine_is_compatible("nvidia,tegra20"))
+   call_firmware_op(l2x0_init);
+
+   if (IS_ENABLED(CONFIG_CACHE_L2X0) &&
+   IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) &&
+   of_machine_is_compatible("nvidia,tegra30"))
+   call_firmware_op(l2x0_init);
+}
+
 static void __init tegra_init_early(void)
 {
of_register_trusted_foundations();
+   tegra_trusted_foundations_l2x0_cache_init();
tegra_cpu_reset_handler_init();
 }
 
-- 
2.17.0

[PATCH v1 1/5] ARM: trusted_foundations: Implement L2 cache initialization callback

[Dmitry Osipenko]

Implement L2 cache initialization firmware callback that should be invoked
early in boot to enable cache HW.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/firmware/trusted_foundations.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/firmware/trusted_foundations.c 
b/arch/arm/firmware/trusted_foundations.c
index 3fb1b5a1dce9..198ce5c75ca0 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -18,8 +18,13 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
+#define TF_CACHE_MAINT 0xf100
+
+#define TF_CACHE_INIT  1
+
 #define TF_SET_CPU_BOOT_ADDR_SMC 0xf200
 
 #define TF_CPU_PM  0xfffc
@@ -63,9 +68,27 @@ static int tf_prepare_idle(void)
return 0;
 }
 
+#ifdef CONFIG_CACHE_L2X0
+static void tf_cache_write_sec(unsigned long val, unsigned int reg)
+{
+   pr_warn("%s: Ignoring write [0x%x]: 0x%08lx\n", __func__, reg, val);
+}
+
+static int tf_init_cache(void)
+{
+   outer_cache.write_sec = tf_cache_write_sec;
+   tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_INIT, 0);
+
+   return 0;
+}
+#endif /* CONFIG_CACHE_L2X0 */
+
 static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
.prepare_idle = tf_prepare_idle,
+#ifdef CONFIG_CACHE_L2X0
+   .l2x0_init = tf_init_cache,
+#endif
 };
 
 void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
-- 
2.17.0

Re: [PATCH v1 1/5] ARM: trusted_foundations: Implement L2 cache initialization callback

[Dmitry Osipenko]

On 20.05.2018 17:08, Russell King - ARM Linux wrote:
> On Sun, May 20, 2018 at 01:15:38PM +0300, Dmitry Osipenko wrote:
>> Implement L2 cache initialization firmware callback that should be invoked
>> early in boot to enable cache HW.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>  arch/arm/firmware/trusted_foundations.c | 23 +++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/arch/arm/firmware/trusted_foundations.c 
>> b/arch/arm/firmware/trusted_foundations.c
>> index 3fb1b5a1dce9..198ce5c75ca0 100644
>> --- a/arch/arm/firmware/trusted_foundations.c
>> +++ b/arch/arm/firmware/trusted_foundations.c
>> @@ -18,8 +18,13 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  
>> +#define TF_CACHE_MAINT  0xf100
>> +
>> +#define TF_CACHE_INIT   1
>> +
>>  #define TF_SET_CPU_BOOT_ADDR_SMC 0xf200
>>  
>>  #define TF_CPU_PM   0xfffc
>> @@ -63,9 +68,27 @@ static int tf_prepare_idle(void)
>>  return 0;
>>  }
>>  
>> +#ifdef CONFIG_CACHE_L2X0
>> +static void tf_cache_write_sec(unsigned long val, unsigned int reg)
>> +{
>> +pr_warn("%s: Ignoring write [0x%x]: 0x%08lx\n", __func__, reg, val);
> 
> Why at warning level?  Is this some issue that the user needs to be
> warned about?
> 

If cache-l2x0 code will be changed in the future in a way that it will try to do
something using the secure-registers, then user should be informed about that
incident as we are ignoring the accesses to secure-registers and this may lead
to an undesired consequences. If a such change in cache-l2x0 will happen, then
we will have to take some action by either fixing the invalid accesses or
silencing the warning message if will be appropriate. For now I'd prefer to have
verbosity in the KMSG to masking the potential problems.

[PATCH v1] media: staging: tegra-vde: Reset memory client

[Dmitry Osipenko]

DMA requests must be blocked before resetting VDE HW, otherwise it is
possible to get a memory corruption or a machine hang. Use the reset
control provided by the Memory Controller to block DMA before resetting
the VDE HW.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 42 +++--
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 90177a59b97c..6dd3bf4481be 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -73,6 +73,7 @@ struct tegra_vde {
struct mutex lock;
struct miscdevice miscdev;
struct reset_control *rst;
+   struct reset_control *rst_mc;
struct gen_pool *iram_pool;
struct completion decode_completion;
struct clk *clk;
@@ -850,9 +851,23 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
 * We rely on the VDE registers reset value, otherwise VDE
 * causes bus lockup.
 */
+   ret = reset_control_assert(vde->rst_mc);
+   if (ret) {
+   dev_err(dev, "DEC start: Failed to assert MC reset: %d\n",
+   ret);
+   goto put_runtime_pm;
+   }
+
ret = reset_control_reset(vde->rst);
if (ret) {
-   dev_err(dev, "Failed to reset HW: %d\n", ret);
+   dev_err(dev, "DEC start: Failed to reset HW: %d\n", ret);
+   goto put_runtime_pm;
+   }
+
+   ret = reset_control_deassert(vde->rst_mc);
+   if (ret) {
+   dev_err(dev, "DEC start: Failed to deassert MC reset: %d\n",
+   ret);
goto put_runtime_pm;
}
 
@@ -880,9 +895,21 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
ret = timeout;
}
 
-   err = reset_control_assert(vde->rst);
-   if (err)
-   dev_err(dev, "Failed to assert HW reset: %d\n", err);
+   /*
+* At first reset memory client to avoid resetting VDE HW in the
+* middle of DMA which could result into memory corruption or hang
+* the whole system.
+*/
+   err = reset_control_assert(vde->rst_mc);
+   if (!err) {
+   err = reset_control_assert(vde->rst);
+   if (err)
+   dev_err(dev,
+   "DEC end: Failed to assert HW reset: %d\n",
+   err);
+   } else {
+   dev_err(dev, "DEC end: Failed to assert MC reset: %d\n", err);
+   }
 
 put_runtime_pm:
pm_runtime_mark_last_busy(dev);
@@ -1074,6 +1101,13 @@ static int tegra_vde_probe(struct platform_device *pdev)
return err;
}
 
+   vde->rst_mc = devm_reset_control_get_optional(dev, "mc");
+   if (IS_ERR(vde->rst_mc)) {
+   err = PTR_ERR(vde->rst_mc);
+   dev_err(dev, "Could not get MC reset %d\n", err);
+   return err;
+   }
+
irq = platform_get_irq_byname(pdev, "sync-token");
if (irq < 0)
return irq;
-- 
2.17.0

Re: [PATCH v2] pinctrl: tegra20: Provide CDEV1/2 clock muxes

[Dmitry Osipenko]

On 16.05.2018 15:23, Linus Walleij wrote:
> On Fri, May 4, 2018 at 12:55 AM, Dmitry Osipenko <dig...@gmail.com> wrote:
> 
>> Muxing of pins MCLK1/2 determine the muxing of the corresponding clocks.
>> Make pinctrl driver to provide clock muxes for the CDEV1/2 pingroups, so
>> that main clk-controller driver could get an actual parent clock for the
>> CDEV1/2 clocks.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> Reviewed-by: Marcel Ziswiler <mar...@ziswiler.com>
>> Tested-by: Marcel Ziswiler <mar...@ziswiler.com>
>> Tested-by: Marc Dietrich <marvi...@gmx.de>
>> Acked-by: Peter De Schrijver <pdeschrij...@nvidia.com>
>> ---
>>
>> v2: This patch is factored out from the v1 clk/DT series so that it could be
>> applied separately.
> 
> Patch applied unless Stephen W protests.
> Please include swarren on future patches to this driver.

I think previously get_maintainer script suggested driver authors, probably it
changed sometime ago. I'm pretty sure that Stephen follows the linux-tegra ML
(Stephen, are you?) and would have jumped into the thread if there was anything
wrong in the patch, though indeed won't hurt to ping him for this patch 
explicitly.

Stephen, please let me know if you disagree with anything in this patch.

Re: [PATCH v1 7/9] iommu/tegra: gart: Provide single domain and group for all devices

[Dmitry Osipenko]

On 14.05.2018 21:18, Robin Murphy wrote:
> On 11/05/18 21:05, Dmitry Osipenko wrote:
>> On 11.05.2018 15:32, Robin Murphy wrote:
>>> On 08/05/18 19:16, Dmitry Osipenko wrote:
>>>> GART aperture is shared by all devices, hence there is a single IOMMU
>>>> domain and group shared by these devices. Allocation of a group per
>>>> device only wastes resources and allowance of having more than one domain
>>>> is simply wrong because IOMMU mappings made by the users of "different"
>>>> domains will stomp on each other.
>>>
>>> Strictly, that reasoning is a bit backwards - allocating multiple groups is 
>>> the
>>> conceptually-wrong thing if the GART cannot differentiate between different
>>> devices, whereas having multiple domains *exist* is no real problem, it's 
>>> merely
>>> that only one can be active at any point in time (which will inherently 
>>> become
>>> the case once all devices are grouped together).
>>
>> IIUC, the IOMMU domain represents the address space. There is only one 
>> address
>> space in a case of GART, the GART's aperture. So GART not only isn't
>> differentiating between different devices, but also between different 
>> domains.
> 
> Right, but that's the same as many other IOMMUs (exynos, rockchip, mtk, etc.) 
> -
> the point is that an IOMMU domain represents *an* address space, but if 
> nothing
> is attached to that domain, it's just a set of logical mappings which doesn't
> need to be backed by real hardware. It's specifically *because* these IOMMUs
> also can't differentiate between devices that things work out neatly - there's
> only one group, which can only be attached to a single domain at once, so 
> there
> is never a time when more than one domain needs to be backed by hardware. 
> Think
> of the IOMMU+devices as a CPU and the domains as processes ;)

\I think\ I understand what you are trying to convey. The "domain swapping"
functionality sounds like a good idea, but I don't see any practical application
to a such functionality right now. Your suggestion also feels a bit like an
implicit ad hoc to me, maybe we could extended IOMMU API to support somewhat
like "explicit domain swapping" for device drivers if multiple platforms will
need that.

In a case of the Tegra-GART driver, I'd prefer to allow having only a single
IOMMU domain in the system for the starter and implement other features on by
as-needed basis.

>>>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>>>> ---
>>>>    drivers/iommu/tegra-gart.c | 107 +
>>>>    1 file changed, 24 insertions(+), 83 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
>>>> index 5b2d27620350..ebc105c201bd 100644
>>>> --- a/drivers/iommu/tegra-gart.c
>>>> +++ b/drivers/iommu/tegra-gart.c
>>>> @@ -19,7 +19,6 @@
>>>>      #include 
>>>>    #include 
>>>> -#include 
>>>>    #include 
>>>>    #include 
>>>>    #include 
>>>> @@ -44,22 +43,17 @@
>>>>    #define GART_PAGE_MASK    \
>>>>    (~(GART_PAGE_SIZE - 1) & ~GART_ENTRY_PHYS_ADDR_VALID)
>>>>    -struct gart_client {
>>>> -    struct device    *dev;
>>>> -    struct list_head    list;
>>>> -};
>>>> -
>>>>    struct gart_device {
>>>>    void __iomem    *regs;
>>>>    u32    *savedata;
>>>>    u32    page_count;    /* total remappable size */
>>>>    dma_addr_t    iovmm_base;    /* offset to vmm_area */
>>>>    spinlock_t    pte_lock;    /* for pagetable */
>>>> -    struct list_head    client;
>>>> -    spinlock_t    client_lock;    /* for client list */
>>>>    struct device    *dev;
>>>>      struct iommu_device    iommu;    /* IOMMU Core handle */
>>>> +    struct iommu_group    *group;    /* Common IOMMU group */
>>>> +    struct gart_domain    *domain;    /* Unique IOMMU domain */
>>>>      struct tegra_mc_gart_handle mc_gart_handle;
>>>>    };
>>>> @@ -169,81 +163,31 @@ static inline bool gart_iova_range_valid(struct
>>>> gart_device *gart,
>>>>    static int gart_iommu_attach_dev(struct iommu_domain *domain,
>>>>     struct device *dev)
>>>>    {
>>>> -    struct gart_domain *gart_dom

[PATCH v1 03/11] cpufreq: tegra20: Remove EMC clock usage

[Dmitry Osipenko]

The EMC driver has been gone 4 years ago, since the commit a7cbe92cef27
("ARM: tegra: remove tegra EMC scaling driver"). Remove the EMC clock
usage as it does nothing. We may consider re-implementing the EMC scaling
later, probably using PM Memory Bandwidth QoS API.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 22 --
 1 file changed, 22 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index dd8a76a64a8e..693f9067ba8a 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -44,7 +44,6 @@ static struct cpufreq_frequency_table freq_table[] = {
 static struct clk *cpu_clk;
 static struct clk *pll_x_clk;
 static struct clk *pll_p_clk;
-static struct clk *emc_clk;
 static bool pll_x_prepared;
 
 static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
@@ -95,17 +94,6 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
int ret = 0;
 
-   /*
-* Vote on memory bus frequency based on cpu frequency
-* This sets the minimum frequency, display or avp may request higher
-*/
-   if (rate >= 816000)
-   clk_set_rate(emc_clk, 6); /* cpu 816 MHz, emc max */
-   else if (rate >= 456000)
-   clk_set_rate(emc_clk, 3); /* cpu 456 MHz, emc 150Mhz */
-   else
-   clk_set_rate(emc_clk, 1);  /* emc 50Mhz */
-
/*
 * target freq == pll_p, don't need to take extra reference to pll_x_clk
 * as it isn't used anymore.
@@ -141,14 +129,12 @@ static int tegra_cpu_init(struct cpufreq_policy *policy)
if (policy->cpu >= NUM_CPUS)
return -EINVAL;
 
-   clk_prepare_enable(emc_clk);
clk_prepare_enable(cpu_clk);
 
/* FIXME: what's the actual transition time? */
ret = cpufreq_generic_init(policy, freq_table, 300 * 1000);
if (ret) {
clk_disable_unprepare(cpu_clk);
-   clk_disable_unprepare(emc_clk);
return ret;
}
 
@@ -160,7 +146,6 @@ static int tegra_cpu_init(struct cpufreq_policy *policy)
 static int tegra_cpu_exit(struct cpufreq_policy *policy)
 {
clk_disable_unprepare(cpu_clk);
-   clk_disable_unprepare(emc_clk);
return 0;
 }
 
@@ -192,19 +177,12 @@ static int __init tegra_cpufreq_init(void)
if (IS_ERR(pll_p_clk))
return PTR_ERR(pll_p_clk);
 
-   emc_clk = clk_get_sys("cpu", "emc");
-   if (IS_ERR(emc_clk)) {
-   clk_put(cpu_clk);
-   return PTR_ERR(emc_clk);
-   }
-
return cpufreq_register_driver(_cpufreq_driver);
 }
 
 static void __exit tegra_cpufreq_exit(void)
 {
cpufreq_unregister_driver(_cpufreq_driver);
-   clk_put(emc_clk);
clk_put(cpu_clk);
 }
 
-- 
2.17.0

[PATCH v1 00/11] Clean up Tegra20 cpufreq driver

[Dmitry Osipenko]

Hello,

Recently Peter Geis (who is working on Tegra30 cpufreq driver) asked me how
tegra20-cpufreq driver is getting loaded. After taking a look at the code
it became apparent that the drivers code has been rusted a tad and so this
series is intended to refresh the drivers code by disallowing module to be
loaded on non-Tegra20 machines, by cleaning whitespaces in the code, removing
dead EMC code and in the end by allowing tegra20-cpufreq to be built as a
loadable module.

Please review, thanks.

Dmitry Osipenko (11):
  cpufreq: tegra20: Change module description
  cpufreq: tegra20: Clean up whitespaces in the code
  cpufreq: tegra20: Remove EMC clock usage
  cpufreq: tegra20: Release clocks properly
  cpufreq: tegra20: Clean up included headers
  cpufreq: tegra20: Remove unneeded check in tegra_cpu_init
  cpufreq: tegra20: Check if this is Tegra20 machine
  cpufreq: tegra20: Remove unneeded variable initialization
  cpufreq: tegra20: Allow cpufreq driver to be built as loadable module
  cpufreq: tegra20: Wrap cpufreq into platform driver
  ARM: tegra: Create platform device for tegra20-cpufreq driver

 arch/arm/mach-tegra/tegra.c   |   4 +
 drivers/cpufreq/Kconfig.arm   |   2 +-
 drivers/cpufreq/tegra20-cpufreq.c | 172 +-
 3 files changed, 101 insertions(+), 77 deletions(-)

-- 
2.17.0

[PATCH v1 04/11] cpufreq: tegra20: Release clocks properly

[Dmitry Osipenko]

Properly put requested clocks in the module init/exit code.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 693f9067ba8a..69f033d297e1 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -165,24 +165,45 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
 
 static int __init tegra_cpufreq_init(void)
 {
+   int err;
+
cpu_clk = clk_get_sys(NULL, "cclk");
if (IS_ERR(cpu_clk))
return PTR_ERR(cpu_clk);
 
pll_x_clk = clk_get_sys(NULL, "pll_x");
-   if (IS_ERR(pll_x_clk))
-   return PTR_ERR(pll_x_clk);
+   if (IS_ERR(pll_x_clk)) {
+   err = PTR_ERR(pll_x_clk);
+   goto put_cpu;
+   }
 
pll_p_clk = clk_get_sys(NULL, "pll_p");
-   if (IS_ERR(pll_p_clk))
-   return PTR_ERR(pll_p_clk);
+   if (IS_ERR(pll_p_clk)) {
+   err = PTR_ERR(pll_p_clk);
+   goto put_pll_x;
+   }
+
+   err = cpufreq_register_driver(_cpufreq_driver);
+   if (err)
+   goto put_pll_p;
+
+   return 0;
+
+put_pll_p:
+   clk_put(pll_p_clk);
+put_pll_x:
+   clk_put(pll_x_clk);
+put_cpu:
+   clk_put(cpu_clk);
 
-   return cpufreq_register_driver(_cpufreq_driver);
+   return err;
 }
 
 static void __exit tegra_cpufreq_exit(void)
 {
cpufreq_unregister_driver(_cpufreq_driver);
+   clk_put(pll_p_clk);
+   clk_put(pll_x_clk);
clk_put(cpu_clk);
 }
 
-- 
2.17.0

[PATCH v1 05/11] cpufreq: tegra20: Clean up included headers

[Dmitry Osipenko]

Remove unused/unneeded headers and sort them in the alphabet order.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 69f033d297e1..61f00d1cba26 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -16,16 +16,9 @@
  *
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
 #include 
-#include 
+#include 
+#include 
 
 static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = 216000 },
-- 
2.17.0

[PATCH v1 07/11] cpufreq: tegra20: Check if this is Tegra20 machine

[Dmitry Osipenko]

Don't even try to request the clocks during of module initialization on
non-Tegra20 machines (this is the case for a multi-platform kernel) for
consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 147ae3e14f18..797c61c74b65 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = 216000 },
@@ -155,6 +156,9 @@ static int __init tegra_cpufreq_init(void)
 {
int err;
 
+   if (!of_machine_is_compatible("nvidia,tegra20"))
+   return -ENODEV;
+
cpu_clk = clk_get_sys(NULL, "cclk");
if (IS_ERR(cpu_clk))
return PTR_ERR(cpu_clk);
-- 
2.17.0

[PATCH v1 06/11] cpufreq: tegra20: Remove unneeded check in tegra_cpu_init

[Dmitry Osipenko]

Remove checking of the CPU number for consistency as it won't ever fail
unless there is a severe bug in the cpufreq core.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 61f00d1cba26..147ae3e14f18 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -32,8 +32,6 @@ static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = CPUFREQ_TABLE_END },
 };
 
-#define NUM_CPUS   2
-
 static struct clk *cpu_clk;
 static struct clk *pll_x_clk;
 static struct clk *pll_p_clk;
@@ -119,9 +117,6 @@ static int tegra_cpu_init(struct cpufreq_policy *policy)
 {
int ret;
 
-   if (policy->cpu >= NUM_CPUS)
-   return -EINVAL;
-
clk_prepare_enable(cpu_clk);
 
/* FIXME: what's the actual transition time? */
-- 
2.17.0

[PATCH v1 08/11] cpufreq: tegra20: Remove unneeded variable initialization

[Dmitry Osipenko]

Remove unneeded variable initialization solely for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 797c61c74b65..c0a7b5a78aa6 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -84,7 +84,7 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 {
unsigned long rate = freq_table[index].frequency;
unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
-   int ret = 0;
+   int ret;
 
/*
 * target freq == pll_p, don't need to take extra reference to pll_x_clk
-- 
2.17.0

[PATCH v1 09/11] cpufreq: tegra20: Allow cpufreq driver to be built as loadable module

[Dmitry Osipenko]

Nothing prevents Tegra20 CPUFreq module to be unloaded, hence allow it to
be built as a non-builtin kernel module.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/Kconfig.arm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 96b35b8b3606..a8a2e210c624 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -264,7 +264,7 @@ config ARM_TANGO_CPUFREQ
default y
 
 config ARM_TEGRA20_CPUFREQ
-   bool "Tegra20 CPUFreq support"
+   tristate "Tegra20 CPUFreq support"
depends on ARCH_TEGRA
default y
help
-- 
2.17.0

[PATCH v1 02/11] cpufreq: tegra20: Clean up whitespaces in the code

[Dmitry Osipenko]

Remove unneeded blank line and replace whitespaces with a tab in the code
for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 10793498355e..dd8a76a64a8e 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -203,12 +203,11 @@ static int __init tegra_cpufreq_init(void)
 
 static void __exit tegra_cpufreq_exit(void)
 {
-cpufreq_unregister_driver(_cpufreq_driver);
+   cpufreq_unregister_driver(_cpufreq_driver);
clk_put(emc_clk);
clk_put(cpu_clk);
 }
 
-
 MODULE_AUTHOR("Colin Cross <ccr...@android.com>");
 MODULE_DESCRIPTION("NVIDIA Tegra20 cpufreq driver");
 MODULE_LICENSE("GPL");
-- 
2.17.0

[PATCH v1 10/11] cpufreq: tegra20: Wrap cpufreq into platform driver

[Dmitry Osipenko]

Currently tegra20-cpufreq kernel module isn't getting autoloaded because
there is no device associated with the module, this is one of two patches
that resolves the module autoloading. This patch adds a module alias that
will associate the tegra20-cpufreq kernel module with the platform device,
other patch will instantiate the actual platform device. And now it makes
sense to wrap cpufreq driver into a platform driver for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 116 +++---
 1 file changed, 73 insertions(+), 43 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index c0a7b5a78aa6..f9d02a28df9e 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -19,7 +19,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = 216000 },
@@ -33,15 +33,19 @@ static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = CPUFREQ_TABLE_END },
 };
 
-static struct clk *cpu_clk;
-static struct clk *pll_x_clk;
-static struct clk *pll_p_clk;
-static bool pll_x_prepared;
+struct tegra20_cpufreq_data {
+   struct device *dev;
+   struct clk *cpu_clk;
+   struct clk *pll_x_clk;
+   struct clk *pll_p_clk;
+   bool pll_x_prepared;
+};
 
 static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
   unsigned int index)
 {
-   unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
+   struct tegra20_cpufreq_data *data = cpufreq_get_driver_data();
+   unsigned int ifreq = clk_get_rate(data->pll_p_clk) / 1000;
 
/*
 * Don't switch to intermediate freq if:
@@ -57,6 +61,7 @@ static unsigned int tegra_get_intermediate(struct 
cpufreq_policy *policy,
 static int tegra_target_intermediate(struct cpufreq_policy *policy,
 unsigned int index)
 {
+   struct tegra20_cpufreq_data *data = cpufreq_get_driver_data();
int ret;
 
/*
@@ -69,21 +74,22 @@ static int tegra_target_intermediate(struct cpufreq_policy 
*policy,
 * Also, we wouldn't be using pll_x anymore and must not take extra
 * reference to it, as it can be disabled now to save some power.
 */
-   clk_prepare_enable(pll_x_clk);
+   clk_prepare_enable(data->pll_x_clk);
 
-   ret = clk_set_parent(cpu_clk, pll_p_clk);
+   ret = clk_set_parent(data->cpu_clk, data->pll_p_clk);
if (ret)
-   clk_disable_unprepare(pll_x_clk);
+   clk_disable_unprepare(data->pll_x_clk);
else
-   pll_x_prepared = true;
+   data->pll_x_prepared = true;
 
return ret;
 }
 
 static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
 {
+   struct tegra20_cpufreq_data *data = cpufreq_get_driver_data();
unsigned long rate = freq_table[index].frequency;
-   unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
+   unsigned int ifreq = clk_get_rate(data->pll_p_clk) / 1000;
int ret;
 
/*
@@ -91,14 +97,14 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 * as it isn't used anymore.
 */
if (rate == ifreq)
-   return clk_set_parent(cpu_clk, pll_p_clk);
+   return clk_set_parent(data->cpu_clk, data->pll_p_clk);
 
-   ret = clk_set_rate(pll_x_clk, rate * 1000);
+   ret = clk_set_rate(data->pll_x_clk, rate * 1000);
/* Restore to earlier frequency on error, i.e. pll_x */
if (ret)
-   pr_err("Failed to change pll_x to %lu\n", rate);
+   dev_err(data->dev, "Failed to change pll_x to %lu\n", rate);
 
-   ret = clk_set_parent(cpu_clk, pll_x_clk);
+   ret = clk_set_parent(data->cpu_clk, data->pll_x_clk);
/* This shouldn't fail while changing or restoring */
WARN_ON(ret);
 
@@ -106,9 +112,9 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 * Drop count to pll_x clock only if we switched to intermediate freq
 * earlier while transitioning to a target frequency.
 */
-   if (pll_x_prepared) {
-   clk_disable_unprepare(pll_x_clk);
-   pll_x_prepared = false;
+   if (data->pll_x_prepared) {
+   clk_disable_unprepare(data->pll_x_clk);
+   data->pll_x_prepared = false;
}
 
return ret;
@@ -116,25 +122,28 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 
 static int tegra_cpu_init(struct cpufreq_policy *policy)
 {
+   struct tegra20_cpufreq_data *data = cpufreq_get_driver_data();
int ret;
 
-   clk_prepare_enable(cpu_clk);
+   clk_prepare_enable(data->cp

[PATCH v1 11/11] ARM: tegra: Create platform device for tegra20-cpufreq driver

[Dmitry Osipenko]

Tegra20-cpufreq driver require a platform device in order to be loaded,
instantiate a simple platform device for the driver during of the machines
late initialization.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 arch/arm/mach-tegra/tegra.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 02e712d2ea30..f9587be48235 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -97,6 +97,10 @@ static void __init tegra_dt_init_late(void)
if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
of_machine_is_compatible("compal,paz00"))
tegra_paz00_wifikill_init();
+
+   if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
+   of_machine_is_compatible("nvidia,tegra20"))
+   platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
 }
 
 static const char * const tegra_dt_board_compat[] = {
-- 
2.17.0

[PATCH v1 01/11] cpufreq: tegra20: Change module description

[Dmitry Osipenko]

Change module description to be in line with the other Tegra drivers, just
for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 2bd62845e9d5..10793498355e 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -210,7 +210,7 @@ static void __exit tegra_cpufreq_exit(void)
 
 
 MODULE_AUTHOR("Colin Cross <ccr...@android.com>");
-MODULE_DESCRIPTION("cpufreq driver for Nvidia Tegra2");
+MODULE_DESCRIPTION("NVIDIA Tegra20 cpufreq driver");
 MODULE_LICENSE("GPL");
 module_init(tegra_cpufreq_init);
 module_exit(tegra_cpufreq_exit);
-- 
2.17.0

Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

[Re-sending the review I made before of the series RESEND]

> Hello Stefan,
> 
> I don't have expertise to review the actual NAND-related driver logic, so I 
> only
> reviewed the basics. The driver code looks good to me, though I've couple 
> minor
> comments.
> 
> On 21.05.2018 03:16, Stefan Agner wrote:
>> Add support for the NAND flash controller found on NVIDIA
>> Tegra 2 SoCs. This implementation does not make use of the
>> command queue feature. Regular operations/data transfers are
>> done in PIO mode. Page read/writes with hardware ECC make
>> use of the DMA for data transfer.
>> 
>> Signed-off-by: Lucas Stach 
>> Signed-off-by: Stefan Agner 
>> ---
>>  MAINTAINERS   |   7 +
>>  drivers/mtd/nand/raw/Kconfig  |   6 +
>>  drivers/mtd/nand/raw/Makefile |   1 +
>>  drivers/mtd/nand/raw/tegra_nand.c | 915 ++
>>  4 files changed, 929 insertions(+)
>>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 58b9861ccf99..a65739681279 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13844,6 +13844,13 @@ M:  Laxman Dewangan 
>>  S:  Supported
>>  F:  drivers/input/keyboard/tegra-kbc.c
>>  
>> +TEGRA NAND DRIVER
>> +M:  Stefan Agner 
>> +M:  Lucas Stach 
>> +S:  Maintained
>> +F:  Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
>> +F:  drivers/mtd/nand/tegra_nand.c
>> +
>>  TEGRA PWM DRIVER
>>  M:  Thierry Reding 
>>  S:  Supported
>> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
>> index 19a2b283fbbe..bd56264233ca 100644
>> --- a/drivers/mtd/nand/raw/Kconfig
>> +++ b/drivers/mtd/nand/raw/Kconfig
>> @@ -534,4 +534,10 @@ config MTD_NAND_MTK
>>Enables support for NAND controller on MTK SoCs.
>>This controller is found on mt27xx, mt81xx, mt65xx SoCs.
>>  
>> +config MTD_NAND_TEGRA
>> +tristate "Support for NAND on NVIDIA Tegra"
>> +depends on ARCH_TEGRA
>> +help
>> +  Enables support for NAND flash on NVIDIA Tegra SoC based boards.
>> +
>>  endif # MTD_NAND
>> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
>> index 165b7ef9e9a1..d5a5f9832b88 100644
>> --- a/drivers/mtd/nand/raw/Makefile
>> +++ b/drivers/mtd/nand/raw/Makefile
>> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504) += 
>> hisi504_nand.o
>>  obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/
>>  obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o
>>  obj-$(CONFIG_MTD_NAND_MTK)  += mtk_ecc.o mtk_nand.o
>> +obj-$(CONFIG_MTD_NAND_TEGRA)+= tegra_nand.o
>>  
>>  nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
>>  nand-objs += nand_amd.o
>> diff --git a/drivers/mtd/nand/raw/tegra_nand.c 
>> b/drivers/mtd/nand/raw/tegra_nand.c
>> new file mode 100644
>> index ..fa236e683fb8
>> --- /dev/null
>> +++ b/drivers/mtd/nand/raw/tegra_nand.c
>> @@ -0,0 +1,915 @@
>> +/*
>> + * Copyright (C) 2018 Stefan Agner 
>> + * Copyright (C) 2014-2015 Lucas Stach 
>> + * Copyright (C) 2012 Avionic Design GmbH
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include > +#include 
>> +
>> +#define CMD 0x00
>> +#define   CMD_GO(1 << 31)
>> +#define   CMD_CLE   (1 << 30)
>> +#define   CMD_ALE   (1 << 29)
>> +#define   CMD_PIO   (1 << 28)
>> +#define   CMD_TX(1 << 27)
>> +#define   CMD_RX(1 << 26)
>> +#define   CMD_SEC_CMD   (1 << 25)
>> +#define   CMD_AFT_DAT   (1 << 24)
>> +#define   CMD_TRANS_SIZE(x) (((x - 1) & 0xf) << 20)
>> +#define   CMD_A_VALID   (1 << 19)
>> +#define   CMD_B_VALID   (1 << 18)
>> +#define   CMD_RD_STATUS_CHK (1 << 17)
>> +#define   CMD_RBSY_CHK  (1 << 16)
>> +#define   CMD_CE(x) (1 << (8 + ((x) & 0x7)))
>> +#define   CMD_CLE_SIZE(x)   (((x - 1) & 0x3) << 4)
>> +#define   CMD_ALE_SIZE(x)   (((x - 1) & 0xf) << 0)
>> +
>> +#define STATUS  0x04
>> +
>> +#define ISR 0x08
>> +#define   ISR_CORRFAIL_ERR  (1 << 24)
>> +#define   ISR_UND   (1 << 7)
>> +#define   ISR_OVR

Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On 22.05.2018 15:19, Stefan Agner wrote:
> [review sent to my first patch sent off-ml, moving to ml thread]
> 
> On 21.05.2018 16:05, Dmitry Osipenko wrote:
>> Hello Stefan,
>>
>> I don't have expertise to review the actual NAND-related driver logic, so I 
>> only
>> reviewed the basics. The driver code looks good to me, though I've couple 
>> minor
>> comments.
>>
>> On 21.05.2018 03:16, Stefan Agner wrote:
>>> Add support for the NAND flash controller found on NVIDIA
>>> Tegra 2 SoCs. This implementation does not make use of the
>>> command queue feature. Regular operations/data transfers are
>>> done in PIO mode. Page read/writes with hardware ECC make
>>> use of the DMA for data transfer.
>>>
>>> Signed-off-by: Lucas Stach <d...@lynxeye.de>
>>> Signed-off-by: Stefan Agner <ste...@agner.ch>
>>> ---
>>>  MAINTAINERS   |   7 +
>>>  drivers/mtd/nand/raw/Kconfig  |   6 +
>>>  drivers/mtd/nand/raw/Makefile |   1 +
>>>  drivers/mtd/nand/raw/tegra_nand.c | 915 ++
>>>  4 files changed, 929 insertions(+)
>>>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 58b9861ccf99..a65739681279 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -13844,6 +13844,13 @@ M: Laxman Dewangan <ldewan...@nvidia.com>
>>>  S: Supported
>>>  F: drivers/input/keyboard/tegra-kbc.c
>>>
>>> +TEGRA NAND DRIVER
>>> +M: Stefan Agner <ste...@agner.ch>
>>> +M: Lucas Stach <d...@lynxeye.de>
>>> +S: Maintained
>>> +F: Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
>>> +F: drivers/mtd/nand/tegra_nand.c
>>> +
>>>  TEGRA PWM DRIVER
>>>  M: Thierry Reding <thierry.red...@gmail.com>
>>>  S: Supported
>>> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
>>> index 19a2b283fbbe..bd56264233ca 100644
>>> --- a/drivers/mtd/nand/raw/Kconfig
>>> +++ b/drivers/mtd/nand/raw/Kconfig
>>> @@ -534,4 +534,10 @@ config MTD_NAND_MTK
>>>   Enables support for NAND controller on MTK SoCs.
>>>   This controller is found on mt27xx, mt81xx, mt65xx SoCs.
>>>
>>> +config MTD_NAND_TEGRA
>>> +   tristate "Support for NAND on NVIDIA Tegra"
>>> +   depends on ARCH_TEGRA
>>> +   help
>>> + Enables support for NAND flash on NVIDIA Tegra SoC based boards.
>>> +
>>>  endif # MTD_NAND
>>> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
>>> index 165b7ef9e9a1..d5a5f9832b88 100644
>>> --- a/drivers/mtd/nand/raw/Makefile
>>> +++ b/drivers/mtd/nand/raw/Makefile
>>> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)+= 
>>> hisi504_nand.o
>>>  obj-$(CONFIG_MTD_NAND_BRCMNAND)+= brcmnand/
>>>  obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o
>>>  obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o
>>> +obj-$(CONFIG_MTD_NAND_TEGRA)   += tegra_nand.o
>>>
>>>  nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
>>>  nand-objs += nand_amd.o
>>> diff --git a/drivers/mtd/nand/raw/tegra_nand.c 
>>> b/drivers/mtd/nand/raw/tegra_nand.c
>>> new file mode 100644
>>> index ..fa236e683fb8
>>> --- /dev/null
>>> +++ b/drivers/mtd/nand/raw/tegra_nand.c
>>> @@ -0,0 +1,915 @@
>>> +/*
>>> + * Copyright (C) 2018 Stefan Agner <ste...@agner.ch>
>>> + * Copyright (C) 2014-2015 Lucas Stach <d...@lynxeye.de>
>>> + * Copyright (C) 2012 Avionic Design GmbH
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include > +#include 
>>> +
>>> +#define CMD0x00
>>> +#define   CMD_GO   (1 << 31)
>>> +#define   CMD_CLE  (1 << 3

Re: [PATCH v1] cpufreq: tegra20: Fix imbalanced clock enable count

[Dmitry Osipenko]

On 23.05.2018 08:58, Viresh Kumar wrote:
> On 23-05-18, 00:14, Dmitry Osipenko wrote:
>> Tegra20-cpufreq driver missed enabling the CPU clocks. This results in a
>> clock-enable refcount disbalance on PLL_P <-> PLL_X reparent, causing
>> PLL_X to get disabled while it shouldn't. Fix this by enabling the clocks
>> on the driver probe.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>
>> CPUFreq maintainers,
>>
>> Please take into account that this patch is made on top of my recent
>> series of patches [0] "Clean up Tegra20 cpufreq driver" that was fully
>> reviewed, but seems not applied yet. Let me know if you prefer to re-spin
>> the [0], including this patch into the series.
>>
>> [0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=45321
> 
> This is already picked by Rafael and is sitting in pm/bleeding-edge
> branch. Should get merged into linux-next in a day or two.

Neat, thank you for letting me know.

>>  drivers/cpufreq/tegra20-cpufreq.c | 16 +++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>> b/drivers/cpufreq/tegra20-cpufreq.c
>> index 05f57dcd5215..ca5229265b60 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -176,6 +176,14 @@ static int tegra20_cpufreq_probe(struct platform_device 
>> *pdev)
>>  goto put_pll_x;
>>  }
>>  
>> +err = clk_prepare_enable(cpufreq->pll_x_clk);
>> +if (err)
>> +goto put_pll_p;
>> +
>> +err = clk_prepare_enable(cpufreq->pll_p_clk);
>> +if (err)
>> +goto disable_pll_x;
>> +
>>  cpufreq->dev = >dev;
>>  cpufreq->driver.get = cpufreq_generic_get;
>>  cpufreq->driver.attr = cpufreq_generic_attr;
>> @@ -192,12 +200,16 @@ static int tegra20_cpufreq_probe(struct 
>> platform_device *pdev)
>>  
>>  err = cpufreq_register_driver(>driver);
>>  if (err)
>> -goto put_pll_p;
>> +goto disable_pll_p;
>>  
>>  platform_set_drvdata(pdev, cpufreq);
>>  
>>  return 0;
>>  
>> +disable_pll_p:
>> +clk_disable_unprepare(cpufreq->pll_p_clk);
>> +disable_pll_x:
>> +clk_disable_unprepare(cpufreq->pll_x_clk);
>>  put_pll_p:
>>  clk_put(cpufreq->pll_p_clk);
>>  put_pll_x:
>> @@ -214,6 +226,8 @@ static int tegra20_cpufreq_remove(struct platform_device 
>> *pdev)
>>  
>>  cpufreq_unregister_driver(>driver);
>>  
>> +clk_disable_unprepare(cpufreq->pll_p_clk);
>> +clk_disable_unprepare(cpufreq->pll_x_clk);
>>  clk_put(cpufreq->pll_p_clk);
>>  clk_put(cpufreq->pll_x_clk);
>>  clk_put(cpufreq->cpu_clk);
> 
> Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
> 

Re: [PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source

[Dmitry Osipenko]

On 24.05.2018 07:30, Viresh Kumar wrote:
> On 23-05-18, 19:00, Dmitry Osipenko wrote:
>> PLL_C is running at 600MHz which is significantly higher than the 216MHz
>> of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
>> running on that PLL. Let's use PLL_C as intermediate clock source, making
>> CPU snappier a tad during of the frequency transition.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>  drivers/cpufreq/tegra20-cpufreq.c | 25 +
>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>> b/drivers/cpufreq/tegra20-cpufreq.c
>> index 3ad6bded6efc..4bf5ba7da40b 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -25,12 +25,13 @@
>>  #include 
>>  
>>  #define PLL_P_FREQ  216000
>> +#define PLL_C_FREQ  60
>>  
>>  static struct cpufreq_frequency_table freq_table[] = {
>>  { .frequency = 216000 },
>>  { .frequency = 312000 },
>>  { .frequency = 456000 },
>> -{ .frequency = 608000 },
>> +{ .frequency = 60 },
>>  { .frequency = 76 },
>>  { .frequency = 816000 },
>>  { .frequency = 912000 },
>> @@ -44,6 +45,7 @@ struct tegra20_cpufreq {
>>  struct clk *cpu_clk;
>>  struct clk *pll_x_clk;
>>  struct clk *pll_p_clk;
>> +struct clk *pll_c_clk;
>>  bool pll_x_prepared;
>>  };
>>  
>> @@ -58,7 +60,10 @@ static unsigned int tegra_get_intermediate(struct 
>> cpufreq_policy *policy,
>>  if (index == 0 || policy->cur == PLL_P_FREQ)
>>  return 0;
>>  
>> -return PLL_P_FREQ;
>> +if (index == 3 || policy->cur == PLL_C_FREQ)
>> +return 0;
> 
> So we can choose between two different intermediate frequencies ? And
> I didn't like the way magic number 3 is used here. Its prone to errors
> and we better use a macro or something else here.
> 
> Like instead of doing index == 3, what about freq_table[index].freq ==
> PLL_C_FREQ ? Same for the previous patch as well.

The frequency is determined by the parent clock of CCLK (CPU clock), we can
choose between different parents for the CCLK. PLL_C as PLL_P and PLL_X are
among the available parents for the CCLK to choose from and there some others.

I don't mind to use freq_table[index].freq, though I'd like to keep compiled
assembly minimal where possible. Hence the freq_table should be made constant to
tell compiler that it doesn't need to emit data fetches for the table values and
could embed the constants into the code where appropriate.

Could we constify the "struct cpufreq_frequency_table" within the cpufreq core?
Seems nothing prevents this (I already tried to constify - there are no
obstacles), unless some cpufreq driver would try to modify
policy->freq_table->... within the cpufreq callback implementation.

Re: [PATCH v1] cpufreq: tegra20: Fix imbalanced clock enable count

[Dmitry Osipenko]

On 23.05.2018 08:58, Viresh Kumar wrote:
> On 23-05-18, 00:14, Dmitry Osipenko wrote:
>> Tegra20-cpufreq driver missed enabling the CPU clocks. This results in a
>> clock-enable refcount disbalance on PLL_P <-> PLL_X reparent, causing
>> PLL_X to get disabled while it shouldn't. Fix this by enabling the clocks
>> on the driver probe.
>>
>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>> ---
>>
>> CPUFreq maintainers,
>>
>> Please take into account that this patch is made on top of my recent
>> series of patches [0] "Clean up Tegra20 cpufreq driver" that was fully
>> reviewed, but seems not applied yet. Let me know if you prefer to re-spin
>> the [0], including this patch into the series.
>>
>> [0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=45321
> 
> This is already picked by Rafael and is sitting in pm/bleeding-edge
> branch. Should get merged into linux-next in a day or two.
> 
>>  drivers/cpufreq/tegra20-cpufreq.c | 16 +++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>> b/drivers/cpufreq/tegra20-cpufreq.c
>> index 05f57dcd5215..ca5229265b60 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -176,6 +176,14 @@ static int tegra20_cpufreq_probe(struct platform_device 
>> *pdev)
>>  goto put_pll_x;
>>  }
>>  
>> +err = clk_prepare_enable(cpufreq->pll_x_clk);
>> +if (err)
>> +goto put_pll_p;
>> +
>> +err = clk_prepare_enable(cpufreq->pll_p_clk);
>> +if (err)
>> +goto disable_pll_x;
>> +
>>  cpufreq->dev = >dev;
>>  cpufreq->driver.get = cpufreq_generic_get;
>>  cpufreq->driver.attr = cpufreq_generic_attr;
>> @@ -192,12 +200,16 @@ static int tegra20_cpufreq_probe(struct 
>> platform_device *pdev)
>>  
>>  err = cpufreq_register_driver(>driver);
>>  if (err)
>> -goto put_pll_p;
>> +goto disable_pll_p;
>>  
>>  platform_set_drvdata(pdev, cpufreq);
>>  
>>  return 0;
>>  
>> +disable_pll_p:
>> +clk_disable_unprepare(cpufreq->pll_p_clk);
>> +disable_pll_x:
>> +clk_disable_unprepare(cpufreq->pll_x_clk);
>>  put_pll_p:
>>  clk_put(cpufreq->pll_p_clk);
>>  put_pll_x:
>> @@ -214,6 +226,8 @@ static int tegra20_cpufreq_remove(struct platform_device 
>> *pdev)
>>  
>>  cpufreq_unregister_driver(>driver);
>>  
>> +clk_disable_unprepare(cpufreq->pll_p_clk);
>> +clk_disable_unprepare(cpufreq->pll_x_clk);
>>  clk_put(cpufreq->pll_p_clk);
>>  clk_put(cpufreq->pll_x_clk);
>>  clk_put(cpufreq->cpu_clk);
> 
> Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
> 

Please hold on this patch, seems I interpreted the cpufreq driver logic
incorrectly and it is probably fine as it is. I'll re-check later today.

[PATCH v1] cpufreq: tegra20: Fix imbalanced clock enable count

[Dmitry Osipenko]

Tegra20-cpufreq driver missed enabling the CPU clocks. This results in a
clock-enable refcount disbalance on PLL_P <-> PLL_X reparent, causing
PLL_X to get disabled while it shouldn't. Fix this by enabling the clocks
on the driver probe.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---

CPUFreq maintainers,

Please take into account that this patch is made on top of my recent
series of patches [0] "Clean up Tegra20 cpufreq driver" that was fully
reviewed, but seems not applied yet. Let me know if you prefer to re-spin
the [0], including this patch into the series.

[0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=45321

 drivers/cpufreq/tegra20-cpufreq.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 05f57dcd5215..ca5229265b60 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -176,6 +176,14 @@ static int tegra20_cpufreq_probe(struct platform_device 
*pdev)
goto put_pll_x;
}
 
+   err = clk_prepare_enable(cpufreq->pll_x_clk);
+   if (err)
+   goto put_pll_p;
+
+   err = clk_prepare_enable(cpufreq->pll_p_clk);
+   if (err)
+   goto disable_pll_x;
+
cpufreq->dev = >dev;
cpufreq->driver.get = cpufreq_generic_get;
cpufreq->driver.attr = cpufreq_generic_attr;
@@ -192,12 +200,16 @@ static int tegra20_cpufreq_probe(struct platform_device 
*pdev)
 
err = cpufreq_register_driver(>driver);
if (err)
-   goto put_pll_p;
+   goto disable_pll_p;
 
platform_set_drvdata(pdev, cpufreq);
 
return 0;
 
+disable_pll_p:
+   clk_disable_unprepare(cpufreq->pll_p_clk);
+disable_pll_x:
+   clk_disable_unprepare(cpufreq->pll_x_clk);
 put_pll_p:
clk_put(cpufreq->pll_p_clk);
 put_pll_x:
@@ -214,6 +226,8 @@ static int tegra20_cpufreq_remove(struct platform_device 
*pdev)
 
cpufreq_unregister_driver(>driver);
 
+   clk_disable_unprepare(cpufreq->pll_p_clk);
+   clk_disable_unprepare(cpufreq->pll_x_clk);
clk_put(cpufreq->pll_p_clk);
clk_put(cpufreq->pll_x_clk);
clk_put(cpufreq->cpu_clk);
-- 
2.17.0

Re: [PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source

[Dmitry Osipenko]

On 25.05.2018 11:36, Rafael J. Wysocki wrote:
> On Fri, May 25, 2018 at 10:14 AM, Rafael J. Wysocki <raf...@kernel.org> wrote:
>> On Thu, May 24, 2018 at 2:28 PM, Dmitry Osipenko <dig...@gmail.com> wrote:
>>> On 24.05.2018 11:01, Rafael J. Wysocki wrote:
>>>> On Thu, May 24, 2018 at 7:37 AM, Dmitry Osipenko <dig...@gmail.com> wrote:
>>>>> On 24.05.2018 07:30, Viresh Kumar wrote:
>>>>>> On 23-05-18, 19:00, Dmitry Osipenko wrote:
>>>>>>> PLL_C is running at 600MHz which is significantly higher than the 216MHz
>>>>>>> of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
>>>>>>> running on that PLL. Let's use PLL_C as intermediate clock source, 
>>>>>>> making
>>>>>>> CPU snappier a tad during of the frequency transition.
>>>>>>>
>>>>>>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>>>>>>> ---
>>>>>>>  drivers/cpufreq/tegra20-cpufreq.c | 25 +
>>>>>>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>>>>>>> b/drivers/cpufreq/tegra20-cpufreq.c
>>>>>>> index 3ad6bded6efc..4bf5ba7da40b 100644
>>>>>>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>>>>>>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>>>>>>> @@ -25,12 +25,13 @@
>>>>>>>  #include 
>>>>>>>
>>>>>>>  #define PLL_P_FREQ  216000
>>>>>>> +#define PLL_C_FREQ  60
>>>>>>>
>>>>>>>  static struct cpufreq_frequency_table freq_table[] = {
>>>>>>>  { .frequency = 216000 },
>>>>>>>  { .frequency = 312000 },
>>>>>>>  { .frequency = 456000 },
>>>>>>> -{ .frequency = 608000 },
>>>>>>> +{ .frequency = 60 },
>>>>>>>  { .frequency = 76 },
>>>>>>>  { .frequency = 816000 },
>>>>>>>  { .frequency = 912000 },
>>>>>>> @@ -44,6 +45,7 @@ struct tegra20_cpufreq {
>>>>>>>  struct clk *cpu_clk;
>>>>>>>  struct clk *pll_x_clk;
>>>>>>>  struct clk *pll_p_clk;
>>>>>>> +struct clk *pll_c_clk;
>>>>>>>  bool pll_x_prepared;
>>>>>>>  };
>>>>>>>
>>>>>>> @@ -58,7 +60,10 @@ static unsigned int tegra_get_intermediate(struct 
>>>>>>> cpufreq_policy *policy,
>>>>>>>  if (index == 0 || policy->cur == PLL_P_FREQ)
>>>>>>>  return 0;
>>>>>>>
>>>>>>> -return PLL_P_FREQ;
>>>>>>> +if (index == 3 || policy->cur == PLL_C_FREQ)
>>>>>>> +return 0;
>>>>>>
>>>>>> So we can choose between two different intermediate frequencies ? And
>>>>>> I didn't like the way magic number 3 is used here. Its prone to errors
>>>>>> and we better use a macro or something else here.
>>>>>>
>>>>>> Like instead of doing index == 3, what about freq_table[index].freq ==
>>>>>> PLL_C_FREQ ? Same for the previous patch as well.
>>>>>
>>>>> The frequency is determined by the parent clock of CCLK (CPU clock), we 
>>>>> can
>>>>> choose between different parents for the CCLK. PLL_C as PLL_P and PLL_X 
>>>>> are
>>>>> among the available parents for the CCLK to choose from and there some 
>>>>> others.
>>>>>
>>>>> I don't mind to use freq_table[index].freq, though I'd like to keep 
>>>>> compiled
>>>>> assembly minimal where possible. Hence the freq_table should be made 
>>>>> constant to
>>>>> tell compiler that it doesn't need to emit data fetches for the table 
>>>>> values and
>>>>> could embed the constants into the code where appropriate.
>>>>>
>>>>> Could we constify the "struct cpufreq_frequency_table" within the cpufreq 
>>>>> core?
>>>>> Seems nothing prevents this (I already tried to constify - there are no
>>>>> obstacles), unless some cpufreq driver would try to modify
>>>>> policy->freq_table->... within the cpufreq callback implementation.
>>>>
>>>> Some drivers generate frequency tables out of external data
>>>> unavailable at compile time, like ACPI tables.
>>>
>>> Instead of making the table constant itself (with its values), seems we can 
>>> just
>>> make the policy->freq_table pointer constant. I'll try to make a patch for 
>>> that,
>>> adjusting the pointers in cpufreq core and the drivers. This works for the
>>> acpi-cpufreq at least.
>>
>> Honestly, messing up with the whole subsystem in order to avoid an
>> explicit pointer case doesn't sound right to me.
> 
> Actually, on a second thought I agree that it is better to do it as
> you suggested: make the policy->freq_table pointer constant
> everywhere.
> 
> Sorry for the noise.

No worries.

As I wrote in the reply to the other patch, the Tegra30 support is now on the
way. These changes will collide a tad with the support integration, so I'll
return to re-considering the changes made in this patchset after Tegra30 support
will land. Thank you very much for your reviews and suggestions, I'll take them
into account in the next iteration.

Re: [PATCH] memory: tegra: include linux/spinlock.h

[Dmitry Osipenko]

On 26.05.2018 00:11, Arnd Bergmann wrote:
> The newly added code in this file causes a build error unless
> we include linux/spinlock.h, at least in some configurations:
> 
> In file included from drivers/memory/tegra/mc.h:15,
>  from drivers/memory/tegra/tegra20.c:11:
> include/soc/tegra/mc.h:159:2: error: unknown type name 'spinlock_t'
>   spinlock_t lock;
>   ^~
> drivers/memory/tegra/tegra20.c: In function 'terga20_mc_hotreset_assert':
> drivers/memory/tegra/tegra20.c:207:2: error: implicit declaration of function 
> 'spin_lock_irqsave'; did you mean 'arch_local_irq_save'? 
> [-Werror=implicit-function-declaration]
>   spin_lock_irqsave(>lock, flags);
>   ^
> 
> Fixes: cb557757e1aa ("memory: tegra: Add Tegra20 memory controller hot 
> resets")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> ---
>  drivers/memory/tegra/tegra20.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/memory/tegra/tegra20.c b/drivers/memory/tegra/tegra20.c
> index 7119e532471c..8853e21b773f 100644
> --- a/drivers/memory/tegra/tegra20.c
> +++ b/drivers/memory/tegra/tegra20.c
> @@ -6,6 +6,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include 
>  #include 
>  
>  #include "mc.h"

Yes, the spinlock header was getting included indirectly.

Reviewed-by: Dmitry Osipenko <dig...@gmail.com>

Re: [PATCH v1] media: staging: tegra-vde: Reset memory client

[Dmitry Osipenko]

On 20.05.2018 16:48, Dmitry Osipenko wrote:
> DMA requests must be blocked before resetting VDE HW, otherwise it is
> possible to get a memory corruption or a machine hang. Use the reset
> control provided by the Memory Controller to block DMA before resetting
> the VDE HW.
> 
> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
> ---
>  drivers/staging/media/tegra-vde/tegra-vde.c | 42 +++--
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
> b/drivers/staging/media/tegra-vde/tegra-vde.c
> index 90177a59b97c..6dd3bf4481be 100644
> --- a/drivers/staging/media/tegra-vde/tegra-vde.c
> +++ b/drivers/staging/media/tegra-vde/tegra-vde.c
> @@ -73,6 +73,7 @@ struct tegra_vde {
>   struct mutex lock;
>   struct miscdevice miscdev;
>   struct reset_control *rst;
> + struct reset_control *rst_mc;
>   struct gen_pool *iram_pool;
>   struct completion decode_completion;
>   struct clk *clk;
> @@ -850,9 +851,23 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
> *vde,
>* We rely on the VDE registers reset value, otherwise VDE
>* causes bus lockup.
>*/
> + ret = reset_control_assert(vde->rst_mc);
> + if (ret) {
> + dev_err(dev, "DEC start: Failed to assert MC reset: %d\n",
> + ret);
> + goto put_runtime_pm;
> + }
> +
>   ret = reset_control_reset(vde->rst);
>   if (ret) {
> - dev_err(dev, "Failed to reset HW: %d\n", ret);
> + dev_err(dev, "DEC start: Failed to reset HW: %d\n", ret);
> + goto put_runtime_pm;
> + }
> +
> + ret = reset_control_deassert(vde->rst_mc);
> + if (ret) {
> + dev_err(dev, "DEC start: Failed to deassert MC reset: %d\n",
> + ret);
>   goto put_runtime_pm;
>   }
>  
> @@ -880,9 +895,21 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
> *vde,
>   ret = timeout;
>   }
>  
> - err = reset_control_assert(vde->rst);
> - if (err)
> - dev_err(dev, "Failed to assert HW reset: %d\n", err);
> + /*
> +  * At first reset memory client to avoid resetting VDE HW in the
> +  * middle of DMA which could result into memory corruption or hang
> +  * the whole system.
> +  */
> + err = reset_control_assert(vde->rst_mc);
> + if (!err) {

It occurred to me that there is no need to skip the HW reset if MC resetting
fails. I'll make V2 to change that.

> + err = reset_control_assert(vde->rst);
> + if (err)
> + dev_err(dev,
> + "DEC end: Failed to assert HW reset: %d\n",
> + err);
> + } else {
> + dev_err(dev, "DEC end: Failed to assert MC reset: %d\n", err);
> + }
>  
>  put_runtime_pm:
>   pm_runtime_mark_last_busy(dev);
> @@ -1074,6 +1101,13 @@ static int tegra_vde_probe(struct platform_device 
> *pdev)
>   return err;
>   }
>  
> + vde->rst_mc = devm_reset_control_get_optional(dev, "mc");
> + if (IS_ERR(vde->rst_mc)) {
> + err = PTR_ERR(vde->rst_mc);
> + dev_err(dev, "Could not get MC reset %d\n", err);
> + return err;
> + }
> +
>   irq = platform_get_irq_byname(pdev, "sync-token");
>   if (irq < 0)
>   return irq;
> 

Re: [PATCH v1] cpufreq: tegra20: Fix imbalanced clock enable count

[Dmitry Osipenko]

On 24.05.2018 12:36, Rafael J. Wysocki wrote:
> On Wednesday, May 23, 2018 11:30:39 AM CEST Dmitry Osipenko wrote:
>> On 23.05.2018 08:58, Viresh Kumar wrote:
>>> On 23-05-18, 00:14, Dmitry Osipenko wrote:
>>>> Tegra20-cpufreq driver missed enabling the CPU clocks. This results in a
>>>> clock-enable refcount disbalance on PLL_P <-> PLL_X reparent, causing
>>>> PLL_X to get disabled while it shouldn't. Fix this by enabling the clocks
>>>> on the driver probe.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>>>> ---
>>>>
>>>> CPUFreq maintainers,
>>>>
>>>> Please take into account that this patch is made on top of my recent
>>>> series of patches [0] "Clean up Tegra20 cpufreq driver" that was fully
>>>> reviewed, but seems not applied yet. Let me know if you prefer to re-spin
>>>> the [0], including this patch into the series.
>>>>
>>>> [0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=45321
>>>
>>> This is already picked by Rafael and is sitting in pm/bleeding-edge
>>> branch. Should get merged into linux-next in a day or two.
>>
>> Neat, thank you for letting me know.
> 
> It actually is there in my linux-next branch, but linux-next proper is not
> taking new material this week AFAICS.

I've found it in yours git repo, thank you.

> You'll see this in linux-next on Monday, most probably.
> 

Yes, I noticed that linux-next stopped updating for awhile.

Re: [PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source

[Dmitry Osipenko]

On 24.05.2018 13:04, Peter De Schrijver wrote:
> On Wed, May 23, 2018 at 07:00:20PM +0300, Dmitry Osipenko wrote:
>> PLL_C is running at 600MHz which is significantly higher than the 216MHz
>> of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
>> running on that PLL. Let's use PLL_C as intermediate clock source, making
>> CPU snappier a tad during of the frequency transition.
>>
> 
> pll_c isn't necessarily 600Mhz when used as a source for the second display
> head.

Hmm, indeed.

Even if PLL_C rate will be adjusted, it will be higher than the PLL_P.. won't
it? That's likely to be good enough.

Do you know if any of the available CCLK parents has a glitch-less rate
switching? I.e. CPU won't hang on the rate switch.

There is other possible 600MHz source, the PLL_M. Can we use it? This one also
may become dynamic if we'll consider implementing the memory scaling, but the
memory frequency probably will fit the transition role pretty well.

Re: [PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source

[Dmitry Osipenko]

On 24.05.2018 11:01, Rafael J. Wysocki wrote:
> On Thu, May 24, 2018 at 7:37 AM, Dmitry Osipenko <dig...@gmail.com> wrote:
>> On 24.05.2018 07:30, Viresh Kumar wrote:
>>> On 23-05-18, 19:00, Dmitry Osipenko wrote:
>>>> PLL_C is running at 600MHz which is significantly higher than the 216MHz
>>>> of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
>>>> running on that PLL. Let's use PLL_C as intermediate clock source, making
>>>> CPU snappier a tad during of the frequency transition.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>>>> ---
>>>>  drivers/cpufreq/tegra20-cpufreq.c | 25 +
>>>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>>>> b/drivers/cpufreq/tegra20-cpufreq.c
>>>> index 3ad6bded6efc..4bf5ba7da40b 100644
>>>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>>>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>>>> @@ -25,12 +25,13 @@
>>>>  #include 
>>>>
>>>>  #define PLL_P_FREQ  216000
>>>> +#define PLL_C_FREQ  60
>>>>
>>>>  static struct cpufreq_frequency_table freq_table[] = {
>>>>  { .frequency = 216000 },
>>>>  { .frequency = 312000 },
>>>>  { .frequency = 456000 },
>>>> -{ .frequency = 608000 },
>>>> +{ .frequency = 60 },
>>>>  { .frequency = 76 },
>>>>  { .frequency = 816000 },
>>>>  { .frequency = 912000 },
>>>> @@ -44,6 +45,7 @@ struct tegra20_cpufreq {
>>>>  struct clk *cpu_clk;
>>>>  struct clk *pll_x_clk;
>>>>  struct clk *pll_p_clk;
>>>> +struct clk *pll_c_clk;
>>>>  bool pll_x_prepared;
>>>>  };
>>>>
>>>> @@ -58,7 +60,10 @@ static unsigned int tegra_get_intermediate(struct 
>>>> cpufreq_policy *policy,
>>>>  if (index == 0 || policy->cur == PLL_P_FREQ)
>>>>  return 0;
>>>>
>>>> -return PLL_P_FREQ;
>>>> +if (index == 3 || policy->cur == PLL_C_FREQ)
>>>> +return 0;
>>>
>>> So we can choose between two different intermediate frequencies ? And
>>> I didn't like the way magic number 3 is used here. Its prone to errors
>>> and we better use a macro or something else here.
>>>
>>> Like instead of doing index == 3, what about freq_table[index].freq ==
>>> PLL_C_FREQ ? Same for the previous patch as well.
>>
>> The frequency is determined by the parent clock of CCLK (CPU clock), we can
>> choose between different parents for the CCLK. PLL_C as PLL_P and PLL_X are
>> among the available parents for the CCLK to choose from and there some 
>> others.
>>
>> I don't mind to use freq_table[index].freq, though I'd like to keep compiled
>> assembly minimal where possible. Hence the freq_table should be made 
>> constant to
>> tell compiler that it doesn't need to emit data fetches for the table values 
>> and
>> could embed the constants into the code where appropriate.
>>
>> Could we constify the "struct cpufreq_frequency_table" within the cpufreq 
>> core?
>> Seems nothing prevents this (I already tried to constify - there are no
>> obstacles), unless some cpufreq driver would try to modify
>> policy->freq_table->... within the cpufreq callback implementation.
> 
> Some drivers generate frequency tables out of external data
> unavailable at compile time, like ACPI tables.

Instead of making the table constant itself (with its values), seems we can just
make the policy->freq_table pointer constant. I'll try to make a patch for that,
adjusting the pointers in cpufreq core and the drivers. This works for the
acpi-cpufreq at least.

> But if you know it for the fact that the core doesn't modify the
> frequency table, you could pass a constant table from the driver to
> it, can't you?
> 

Yes, but that will require to explicitly silencing the compiler warning about
const -> non-const pointer conversion (if you're meaning this pointer
conversion), which generally should be avoided.

Re: [PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source

[Dmitry Osipenko]

On 25.05.2018 09:32, Peter De Schrijver wrote:
> On Thu, May 24, 2018 at 03:49:22PM +0300, Dmitry Osipenko wrote:
>> On 24.05.2018 13:04, Peter De Schrijver wrote:
>>> On Wed, May 23, 2018 at 07:00:20PM +0300, Dmitry Osipenko wrote:
>>>> PLL_C is running at 600MHz which is significantly higher than the 216MHz
>>>> of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
>>>> running on that PLL. Let's use PLL_C as intermediate clock source, making
>>>> CPU snappier a tad during of the frequency transition.
>>>>
>>>
>>> pll_c isn't necessarily 600Mhz when used as a source for the second display
>>> head.
>>
>> Hmm, indeed.
>>
>> Even if PLL_C rate will be adjusted, it will be higher than the PLL_P.. won't
>> it? That's likely to be good enough.
>>
> 
> Yes. I think it can be always higher than pll_p, but that assumes the display
> driver will always program the highest possible rate for pll_c for a given 
> mode
> and then program the display divider to divide it down to the required rate.
> 
>> Do you know if any of the available CCLK parents has a glitch-less rate
>> switching? I.e. CPU won't hang on the rate switch.
>>
> 
> Tegra20 doesn't have dynamic ramp PLLs no. So you always have to switch to a
> backup clock source before changing the rate of pll_x.
> 
>> There is other possible 600MHz source, the PLL_M. Can we use it? This one 
>> also
>> may become dynamic if we'll consider implementing the memory scaling, but the
>> memory frequency probably will fit the transition role pretty well.
> 
> I think this should work, but as you mention it may very well be lower than
> pll_p if Tegra20 EMC scaling is re-introduced. I think that's why historically
> this was never done. 
> 
> Peter.
> 

Okay, thank you very much for the input. Let's put on hold these patches then.

Peter Geis just made a cpufreq driver for the Tegra30 and turned out it's just a
dozen lines of code that we'll have to add to the tegra20-cpufreq driver to
support Tegra30. The tegra20-cpufreq driver code suits very well for the Tegra30
integration, use of PLL_C for the intermediate scaling will make that
integration a bit messy. I'll return to considering of the different
transition-PLL variant after adding the Tegra30 support.

[PATCH v2 11/11] cpufreq: tegra20: Wrap cpufreq into platform driver

[Dmitry Osipenko]

Currently tegra20-cpufreq kernel module isn't getting autoloaded because
there is no device associated with the module, this is one of two patches
that resolves the module autoloading. This patch adds a module alias that
will associate the tegra20-cpufreq kernel module with the platform device,
other patch will instantiate the actual platform device. And now it makes
sense to wrap cpufreq driver into a platform driver for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
---
 drivers/cpufreq/tegra20-cpufreq.c | 145 ++
 1 file changed, 86 insertions(+), 59 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 7b425ebe81e7..05f57dcd5215 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -21,7 +21,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 static struct cpufreq_frequency_table freq_table[] = {
@@ -36,15 +36,20 @@ static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = CPUFREQ_TABLE_END },
 };
 
-static struct clk *cpu_clk;
-static struct clk *pll_x_clk;
-static struct clk *pll_p_clk;
-static bool pll_x_prepared;
+struct tegra20_cpufreq {
+   struct device *dev;
+   struct cpufreq_driver driver;
+   struct clk *cpu_clk;
+   struct clk *pll_x_clk;
+   struct clk *pll_p_clk;
+   bool pll_x_prepared;
+};
 
 static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
   unsigned int index)
 {
-   unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
+   struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
+   unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
 
/*
 * Don't switch to intermediate freq if:
@@ -60,6 +65,7 @@ static unsigned int tegra_get_intermediate(struct 
cpufreq_policy *policy,
 static int tegra_target_intermediate(struct cpufreq_policy *policy,
 unsigned int index)
 {
+   struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
int ret;
 
/*
@@ -72,21 +78,22 @@ static int tegra_target_intermediate(struct cpufreq_policy 
*policy,
 * Also, we wouldn't be using pll_x anymore and must not take extra
 * reference to it, as it can be disabled now to save some power.
 */
-   clk_prepare_enable(pll_x_clk);
+   clk_prepare_enable(cpufreq->pll_x_clk);
 
-   ret = clk_set_parent(cpu_clk, pll_p_clk);
+   ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
if (ret)
-   clk_disable_unprepare(pll_x_clk);
+   clk_disable_unprepare(cpufreq->pll_x_clk);
else
-   pll_x_prepared = true;
+   cpufreq->pll_x_prepared = true;
 
return ret;
 }
 
 static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
 {
+   struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
unsigned long rate = freq_table[index].frequency;
-   unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
+   unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
int ret;
 
/*
@@ -94,14 +101,14 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 * as it isn't used anymore.
 */
if (rate == ifreq)
-   return clk_set_parent(cpu_clk, pll_p_clk);
+   return clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
 
-   ret = clk_set_rate(pll_x_clk, rate * 1000);
+   ret = clk_set_rate(cpufreq->pll_x_clk, rate * 1000);
/* Restore to earlier frequency on error, i.e. pll_x */
if (ret)
-   pr_err("Failed to change pll_x to %lu\n", rate);
+   dev_err(cpufreq->dev, "Failed to change pll_x to %lu\n", rate);
 
-   ret = clk_set_parent(cpu_clk, pll_x_clk);
+   ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_x_clk);
/* This shouldn't fail while changing or restoring */
WARN_ON(ret);
 
@@ -109,9 +116,9 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 * Drop count to pll_x clock only if we switched to intermediate freq
 * earlier while transitioning to a target frequency.
 */
-   if (pll_x_prepared) {
-   clk_disable_unprepare(pll_x_clk);
-   pll_x_prepared = false;
+   if (cpufreq->pll_x_prepared) {
+   clk_disable_unprepare(cpufreq->pll_x_clk);
+   cpufreq->pll_x_prepared = false;
}
 
return ret;
@@ -119,91 +126,111 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 
 static int tegra_cpu_init(struct cpufreq_policy *policy)
 {
+   struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
  

[PATCH v2 03/11] cpufreq: tegra20: Clean up included headers

[Dmitry Osipenko]

Remove unused/unneeded headers and sort them in the alphabet order.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index dd8a76a64a8e..bec1a50a8138 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -16,16 +16,12 @@
  *
  */
 
-#include 
-#include 
-#include 
-#include 
+#include 
 #include 
-#include 
-#include 
 #include 
-#include 
-#include 
+#include 
+#include 
+#include 
 
 static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = 216000 },
-- 
2.17.0

[PATCH v2 04/11] cpufreq: tegra20: Remove EMC clock usage

[Dmitry Osipenko]

The EMC driver has been gone 4 years ago, since the commit a7cbe92cef27
("ARM: tegra: remove tegra EMC scaling driver"). Remove the EMC clock
usage as it does nothing. We may consider re-implementing the EMC scaling
later, probably using PM Memory Bandwidth QoS API.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 22 --
 1 file changed, 22 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index bec1a50a8138..4b09a8b410c4 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -40,7 +40,6 @@ static struct cpufreq_frequency_table freq_table[] = {
 static struct clk *cpu_clk;
 static struct clk *pll_x_clk;
 static struct clk *pll_p_clk;
-static struct clk *emc_clk;
 static bool pll_x_prepared;
 
 static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
@@ -91,17 +90,6 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
int ret = 0;
 
-   /*
-* Vote on memory bus frequency based on cpu frequency
-* This sets the minimum frequency, display or avp may request higher
-*/
-   if (rate >= 816000)
-   clk_set_rate(emc_clk, 6); /* cpu 816 MHz, emc max */
-   else if (rate >= 456000)
-   clk_set_rate(emc_clk, 3); /* cpu 456 MHz, emc 150Mhz */
-   else
-   clk_set_rate(emc_clk, 1);  /* emc 50Mhz */
-
/*
 * target freq == pll_p, don't need to take extra reference to pll_x_clk
 * as it isn't used anymore.
@@ -137,14 +125,12 @@ static int tegra_cpu_init(struct cpufreq_policy *policy)
if (policy->cpu >= NUM_CPUS)
return -EINVAL;
 
-   clk_prepare_enable(emc_clk);
clk_prepare_enable(cpu_clk);
 
/* FIXME: what's the actual transition time? */
ret = cpufreq_generic_init(policy, freq_table, 300 * 1000);
if (ret) {
clk_disable_unprepare(cpu_clk);
-   clk_disable_unprepare(emc_clk);
return ret;
}
 
@@ -156,7 +142,6 @@ static int tegra_cpu_init(struct cpufreq_policy *policy)
 static int tegra_cpu_exit(struct cpufreq_policy *policy)
 {
clk_disable_unprepare(cpu_clk);
-   clk_disable_unprepare(emc_clk);
return 0;
 }
 
@@ -188,19 +173,12 @@ static int __init tegra_cpufreq_init(void)
if (IS_ERR(pll_p_clk))
return PTR_ERR(pll_p_clk);
 
-   emc_clk = clk_get_sys("cpu", "emc");
-   if (IS_ERR(emc_clk)) {
-   clk_put(cpu_clk);
-   return PTR_ERR(emc_clk);
-   }
-
return cpufreq_register_driver(_cpufreq_driver);
 }
 
 static void __exit tegra_cpufreq_exit(void)
 {
cpufreq_unregister_driver(_cpufreq_driver);
-   clk_put(emc_clk);
clk_put(cpu_clk);
 }
 
-- 
2.17.0

[PATCH v2 08/11] cpufreq: tegra20: Remove unneeded variable initialization

[Dmitry Osipenko]

Remove unneeded variable initialization solely for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 4b85a6733533..36075aee2ff2 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -86,7 +86,7 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 {
unsigned long rate = freq_table[index].frequency;
unsigned int ifreq = clk_get_rate(pll_p_clk) / 1000;
-   int ret = 0;
+   int ret;
 
/*
 * target freq == pll_p, don't need to take extra reference to pll_x_clk
-- 
2.17.0

[PATCH v2 01/11] cpufreq: tegra20: Change module description

[Dmitry Osipenko]

Change module description to be in line with the other Tegra drivers, just
for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 2bd62845e9d5..10793498355e 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -210,7 +210,7 @@ static void __exit tegra_cpufreq_exit(void)
 
 
 MODULE_AUTHOR("Colin Cross <ccr...@android.com>");
-MODULE_DESCRIPTION("cpufreq driver for Nvidia Tegra2");
+MODULE_DESCRIPTION("NVIDIA Tegra20 cpufreq driver");
 MODULE_LICENSE("GPL");
 module_init(tegra_cpufreq_init);
 module_exit(tegra_cpufreq_exit);
-- 
2.17.0

[PATCH v2 09/11] cpufreq: tegra20: Check if this is Tegra20 machine

[Dmitry Osipenko]

Don't even try to request the clocks during of module initialization on
non-Tegra20 machines (this is the case for a multi-platform kernel) for
consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 36075aee2ff2..7b425ebe81e7 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static struct cpufreq_frequency_table freq_table[] = {
@@ -158,6 +159,9 @@ static int __init tegra_cpufreq_init(void)
 {
int err;
 
+   if (!of_machine_is_compatible("nvidia,tegra20"))
+   return -ENODEV;
+
cpu_clk = clk_get_sys(NULL, "cclk");
if (IS_ERR(cpu_clk))
return PTR_ERR(cpu_clk);
-- 
2.17.0

[PATCH v2 10/11] cpufreq: tegra20: Allow cpufreq driver to be built as loadable module

[Dmitry Osipenko]

Nothing prevents Tegra20 CPUFreq module to be unloaded, hence allow it to
be built as a non-builtin kernel module.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/Kconfig.arm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 96b35b8b3606..a8a2e210c624 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -264,7 +264,7 @@ config ARM_TANGO_CPUFREQ
default y
 
 config ARM_TEGRA20_CPUFREQ
-   bool "Tegra20 CPUFreq support"
+   tristate "Tegra20 CPUFreq support"
depends on ARCH_TEGRA
default y
help
-- 
2.17.0

[PATCH v2 02/11] cpufreq: tegra20: Clean up whitespaces in the code

[Dmitry Osipenko]

Remove unneeded blank line and replace whitespaces with a tab in the code
for consistency.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 10793498355e..dd8a76a64a8e 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -203,12 +203,11 @@ static int __init tegra_cpufreq_init(void)
 
 static void __exit tegra_cpufreq_exit(void)
 {
-cpufreq_unregister_driver(_cpufreq_driver);
+   cpufreq_unregister_driver(_cpufreq_driver);
clk_put(emc_clk);
clk_put(cpu_clk);
 }
 
-
 MODULE_AUTHOR("Colin Cross <ccr...@android.com>");
 MODULE_DESCRIPTION("NVIDIA Tegra20 cpufreq driver");
 MODULE_LICENSE("GPL");
-- 
2.17.0

[PATCH v2 00/11] Clean up Tegra20 cpufreq driver

[Dmitry Osipenko]

Hello,

Recently Peter Geis (who is working on Tegra30 cpufreq driver) asked me how
tegra20-cpufreq driver is getting loaded. After taking a look at the code
it became apparent that the drivers code has been rusted a tad and so this
series is intended to refresh the drivers code by disallowing module to be
loaded on non-Tegra20 machines, by cleaning whitespaces in the code, removing
dead EMC code and in the end by allowing tegra20-cpufreq to be built as a
loadable module.

Changelog:

v2:
- Added the new patch to the series: "Remove unnecessary parentheses",
  please review.

- Addressed review comments to the v1 of the series:

1) Kept undeleted necessary headers in the "Clean up included headers"
   patch as per Viresh's Kumar request.

2) Renamed tegra20_cpufreq_data to tegra20_cpufreq and moved cpufreq
   driver structure to the tegra20_cpufreq as per Thierry's Reding
   request.

- Dropped "ARM: tegra: Create platform device for tegra20-cpufreq 
driver"
  patch from the series as Thierry already applied it and submitted
  in the "ARM: tegra: Core changes for v4.18-rc1" pull request.

Dmitry Osipenko (11):
  cpufreq: tegra20: Change module description
  cpufreq: tegra20: Clean up whitespaces in the code
  cpufreq: tegra20: Clean up included headers
  cpufreq: tegra20: Remove EMC clock usage
  cpufreq: tegra20: Release clocks properly
  cpufreq: tegra20: Remove unneeded check in tegra_cpu_init
  cpufreq: tegra20: Remove unnecessary parentheses
  cpufreq: tegra20: Remove unneeded variable initialization
  cpufreq: tegra20: Check if this is Tegra20 machine
  cpufreq: tegra20: Allow cpufreq driver to be built as loadable module
  cpufreq: tegra20: Wrap cpufreq into platform driver

 drivers/cpufreq/Kconfig.arm   |   2 +-
 drivers/cpufreq/tegra20-cpufreq.c | 200 --
 2 files changed, 111 insertions(+), 91 deletions(-)

-- 
2.17.0

[PATCH v2 07/11] cpufreq: tegra20: Remove unnecessary parentheses

[Dmitry Osipenko]

Remove unnecessary parentheses as suggested by the checkpatch script.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index df25e350c8e6..4b85a6733533 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -50,7 +50,7 @@ static unsigned int tegra_get_intermediate(struct 
cpufreq_policy *policy,
 * - we are already at it, i.e. policy->cur == ifreq
 * - index corresponds to ifreq
 */
-   if ((freq_table[index].frequency == ifreq) || (policy->cur == ifreq))
+   if (freq_table[index].frequency == ifreq || policy->cur == ifreq)
return 0;
 
return ifreq;
-- 
2.17.0

[PATCH v2 06/11] cpufreq: tegra20: Remove unneeded check in tegra_cpu_init

[Dmitry Osipenko]

Remove checking of the CPU number for consistency as it won't ever fail
unless there is a severe bug in the cpufreq core.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index ea186d3f0faf..df25e350c8e6 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -35,8 +35,6 @@ static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = CPUFREQ_TABLE_END },
 };
 
-#define NUM_CPUS   2
-
 static struct clk *cpu_clk;
 static struct clk *pll_x_clk;
 static struct clk *pll_p_clk;
@@ -122,9 +120,6 @@ static int tegra_cpu_init(struct cpufreq_policy *policy)
 {
int ret;
 
-   if (policy->cpu >= NUM_CPUS)
-   return -EINVAL;
-
clk_prepare_enable(cpu_clk);
 
/* FIXME: what's the actual transition time? */
-- 
2.17.0

[PATCH v2 05/11] cpufreq: tegra20: Release clocks properly

[Dmitry Osipenko]

Properly put requested clocks in the module init/exit code.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
Acked-by: Thierry Reding <tred...@nvidia.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 4b09a8b410c4..ea186d3f0faf 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -161,24 +161,45 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
 
 static int __init tegra_cpufreq_init(void)
 {
+   int err;
+
cpu_clk = clk_get_sys(NULL, "cclk");
if (IS_ERR(cpu_clk))
return PTR_ERR(cpu_clk);
 
pll_x_clk = clk_get_sys(NULL, "pll_x");
-   if (IS_ERR(pll_x_clk))
-   return PTR_ERR(pll_x_clk);
+   if (IS_ERR(pll_x_clk)) {
+   err = PTR_ERR(pll_x_clk);
+   goto put_cpu;
+   }
 
pll_p_clk = clk_get_sys(NULL, "pll_p");
-   if (IS_ERR(pll_p_clk))
-   return PTR_ERR(pll_p_clk);
+   if (IS_ERR(pll_p_clk)) {
+   err = PTR_ERR(pll_p_clk);
+   goto put_pll_x;
+   }
+
+   err = cpufreq_register_driver(_cpufreq_driver);
+   if (err)
+   goto put_pll_p;
+
+   return 0;
+
+put_pll_p:
+   clk_put(pll_p_clk);
+put_pll_x:
+   clk_put(pll_x_clk);
+put_cpu:
+   clk_put(cpu_clk);
 
-   return cpufreq_register_driver(_cpufreq_driver);
+   return err;
 }
 
 static void __exit tegra_cpufreq_exit(void)
 {
cpufreq_unregister_driver(_cpufreq_driver);
+   clk_put(pll_p_clk);
+   clk_put(pll_x_clk);
clk_put(cpu_clk);
 }
 
-- 
2.17.0

Re: [PATCH v1] gpu: host1x: Utilize IOMMU mapping for firewall-copied buffers

[Dmitry Osipenko]

On 19.05.2018 02:52, Dmitry Osipenko wrote:
> Map firewall-copied buffers into Host1x's IOVA space, otherwise Host1x
> CDMA can't access the command buffers and all submitted jobs fail if IOMMU
> and Host1x firewall are enabled in the kernels config.
> 
> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
> ---
>  drivers/gpu/host1x/job.c | 58 +++-
>  include/linux/host1x.h   |  4 ++-
>  2 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
> index e2f4a4d93d20..57384a5b5059 100644
> --- a/drivers/gpu/host1x/job.c
> +++ b/drivers/gpu/host1x/job.c
> @@ -449,10 +449,13 @@ static int validate(struct host1x_firewall *fw, struct 
> host1x_job_gather *g)
>  
>  static inline int copy_gathers(struct host1x_job *job, struct device *dev)
>  {
> + struct host1x *host = dev_get_drvdata(job->channel->dev->parent);
>   struct host1x_firewall fw;
> + dma_addr_t dma_addr;
>   size_t size = 0;
>   size_t offset = 0;
>   unsigned int i;
> + int err;
>  
>   fw.job = job;
>   fw.dev = dev;
> @@ -466,23 +469,55 @@ static inline int copy_gathers(struct host1x_job *job, 
> struct device *dev)
>   size += g->words * sizeof(u32);
>   }
>  
> + if (host->domain)
> + size = iova_align(>iova, size);
> +
>   /*
>* Try a non-blocking allocation from a higher priority pools first,
>* as awaiting for the allocation here is a major performance hit.
>*/
> - job->gather_copy_mapped = dma_alloc_wc(dev, size, >gather_copy,
> -GFP_NOWAIT);
> + job->gather_copy_mapped = dma_alloc_wc(dev, size,
> +>gather_copy_phys,
> +GFP_NOWAIT);
>  
>   /* the higher priority allocation failed, try the generic-blocking */
>   if (!job->gather_copy_mapped)
>   job->gather_copy_mapped = dma_alloc_wc(dev, size,
> ->gather_copy,
> +>gather_copy_phys,
>  GFP_KERNEL);
>   if (!job->gather_copy_mapped)
>   return -ENOMEM;
>  
>   job->gather_copy_size = size;
>  
> + if (host->domain) {
> + unsigned long shift;
> +
> + shift = iova_shift(>iova);
> + job->gather_copy_iova_alloc = alloc_iova(
> + >iova, size >> shift,
> + host->iova_end >> shift, true);
> + if (!job->gather_copy_iova_alloc)
> + return -ENOMEM;
> +
> + job->gather_copy_iova = iova_dma_addr(
> + >iova, job->gather_copy_iova_alloc);
> +
> + err = iommu_map(host->domain,
> + job->gather_copy_iova,
> + job->gather_copy_phys,
> + size, IOMMU_READ);
> + if (err) {
> + __free_iova(>iova, job->gather_copy_iova_alloc);
> + job->gather_copy_iova_alloc = NULL;
> + return err;
> + }
> +
> + dma_addr = job->gather_copy_iova;
> + } else {
> + dma_addr = job->gather_copy_phys;
> + }
> +
>   for (i = 0; i < job->num_gathers; i++) {
>   struct host1x_job_gather *g = >gathers[i];
>   void *gather;
> @@ -494,7 +529,7 @@ static inline int copy_gathers(struct host1x_job *job, 
> struct device *dev)
>   host1x_bo_munmap(g->bo, gather);
>  
>   /* Store the location in the buffer */
> - g->base = job->gather_copy;
> + g->base = dma_addr;
>   g->offset = offset;
>  
>   /* Validate the job */
> @@ -582,9 +617,20 @@ void host1x_job_unpin(struct host1x_job *job)
>  
>   job->num_unpins = 0;
>  
> - if (job->gather_copy_size)
> + if (job->gather_copy_size) {
>   dma_free_wc(job->channel->dev, job->gather_copy_size,
> - job->gather_copy_mapped, job->gather_copy);
> + job->gather_copy_mapped, job->gather_copy_phys);
> +
> + if (job->gather_copy_iova_alloc) {
> + iommu_unmap(host->domain,
> + 

[PATCH v1] gpu: host1x: Skip IOMMU initialization if firewall is enabled

[Dmitry Osipenko]

Host1x's CDMA can't access the command buffers if IOMMU and Host1x
firewall are enabled in the kernels config because firewall doesn't map
the copied buffer into IOVA space. Fix this by skipping IOMMU
initialization if firewall is enabled as firewall merges sparse cmdbufs
into a single contiguous buffer and hence IOMMU isn't needed in this case.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/gpu/host1x/dev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index f1d5f76e9c33..d88073e7d22d 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -218,6 +218,9 @@ static int host1x_probe(struct platform_device *pdev)
return err;
}
 
+   if (IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL))
+   goto skip_iommu;
+
host->group = iommu_group_get(>dev);
if (host->group) {
struct iommu_domain_geometry *geometry;
-- 
2.17.0

[PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source

[Dmitry Osipenko]

PLL_C is running at 600MHz which is significantly higher than the 216MHz
of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
running on that PLL. Let's use PLL_C as intermediate clock source, making
CPU snappier a tad during of the frequency transition.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 3ad6bded6efc..4bf5ba7da40b 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -25,12 +25,13 @@
 #include 
 
 #define PLL_P_FREQ 216000
+#define PLL_C_FREQ 60
 
 static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = 216000 },
{ .frequency = 312000 },
{ .frequency = 456000 },
-   { .frequency = 608000 },
+   { .frequency = 60 },
{ .frequency = 76 },
{ .frequency = 816000 },
{ .frequency = 912000 },
@@ -44,6 +45,7 @@ struct tegra20_cpufreq {
struct clk *cpu_clk;
struct clk *pll_x_clk;
struct clk *pll_p_clk;
+   struct clk *pll_c_clk;
bool pll_x_prepared;
 };
 
@@ -58,7 +60,10 @@ static unsigned int tegra_get_intermediate(struct 
cpufreq_policy *policy,
if (index == 0 || policy->cur == PLL_P_FREQ)
return 0;
 
-   return PLL_P_FREQ;
+   if (index == 3 || policy->cur == PLL_C_FREQ)
+   return 0;
+
+   return PLL_C_FREQ;
 }
 
 static int tegra_target_intermediate(struct cpufreq_policy *policy,
@@ -79,7 +84,7 @@ static int tegra_target_intermediate(struct cpufreq_policy 
*policy,
 */
clk_prepare_enable(cpufreq->pll_x_clk);
 
-   ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
+   ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_c_clk);
if (ret)
clk_disable_unprepare(cpufreq->pll_x_clk);
else
@@ -101,6 +106,9 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
if (index == 0)
return clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
 
+   if (index == 3)
+   return clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_c_clk);
+
ret = clk_set_rate(cpufreq->pll_x_clk, rate * 1000);
/* Restore to earlier frequency on error, i.e. pll_x */
if (ret)
@@ -174,6 +182,12 @@ static int tegra20_cpufreq_probe(struct platform_device 
*pdev)
goto put_pll_x;
}
 
+   cpufreq->pll_c_clk = clk_get_sys(NULL, "pll_c");
+   if (IS_ERR(cpufreq->pll_c_clk)) {
+   err = PTR_ERR(cpufreq->pll_c_clk);
+   goto put_pll_p;
+   }
+
cpufreq->dev = >dev;
cpufreq->driver.get = cpufreq_generic_get;
cpufreq->driver.attr = cpufreq_generic_attr;
@@ -190,12 +204,14 @@ static int tegra20_cpufreq_probe(struct platform_device 
*pdev)
 
err = cpufreq_register_driver(>driver);
if (err)
-   goto put_pll_p;
+   goto put_pll_c;
 
platform_set_drvdata(pdev, cpufreq);
 
return 0;
 
+put_pll_c:
+   clk_put(cpufreq->pll_c_clk);
 put_pll_p:
clk_put(cpufreq->pll_p_clk);
 put_pll_x:
@@ -212,6 +228,7 @@ static int tegra20_cpufreq_remove(struct platform_device 
*pdev)
 
cpufreq_unregister_driver(>driver);
 
+   clk_put(cpufreq->pll_c_clk);
clk_put(cpufreq->pll_p_clk);
clk_put(cpufreq->pll_x_clk);
clk_put(cpufreq->cpu_clk);
-- 
2.17.0

[PATCH v1 1/2] cpufreq: tegra20: Constify rate value of the intermediate clk

[Dmitry Osipenko]

PLL_P is known to be always running at 216MHz, hence there is no need to
query its rate.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/cpufreq/tegra20-cpufreq.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
b/drivers/cpufreq/tegra20-cpufreq.c
index 05f57dcd5215..3ad6bded6efc 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -24,6 +24,8 @@
 #include 
 #include 
 
+#define PLL_P_FREQ 216000
+
 static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = 216000 },
{ .frequency = 312000 },
@@ -48,18 +50,15 @@ struct tegra20_cpufreq {
 static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
   unsigned int index)
 {
-   struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
-   unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
-
/*
 * Don't switch to intermediate freq if:
 * - we are already at it, i.e. policy->cur == ifreq
 * - index corresponds to ifreq
 */
-   if (freq_table[index].frequency == ifreq || policy->cur == ifreq)
+   if (index == 0 || policy->cur == PLL_P_FREQ)
return 0;
 
-   return ifreq;
+   return PLL_P_FREQ;
 }
 
 static int tegra_target_intermediate(struct cpufreq_policy *policy,
@@ -93,14 +92,13 @@ static int tegra_target(struct cpufreq_policy *policy, 
unsigned int index)
 {
struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
unsigned long rate = freq_table[index].frequency;
-   unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
int ret;
 
/*
 * target freq == pll_p, don't need to take extra reference to pll_x_clk
 * as it isn't used anymore.
 */
-   if (rate == ifreq)
+   if (index == 0)
return clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
 
ret = clk_set_rate(cpufreq->pll_x_clk, rate * 1000);
-- 
2.17.0

Re: [PATCH v1] cpufreq: tegra20: Fix imbalanced clock enable count

[Dmitry Osipenko]

On 23.05.2018 13:44, Dmitry Osipenko wrote:
> On 23.05.2018 08:58, Viresh Kumar wrote:
>> On 23-05-18, 00:14, Dmitry Osipenko wrote:
>>> Tegra20-cpufreq driver missed enabling the CPU clocks. This results in a
>>> clock-enable refcount disbalance on PLL_P <-> PLL_X reparent, causing
>>> PLL_X to get disabled while it shouldn't. Fix this by enabling the clocks
>>> on the driver probe.
>>>
>>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>>> ---
>>>
>>> CPUFreq maintainers,
>>>
>>> Please take into account that this patch is made on top of my recent
>>> series of patches [0] "Clean up Tegra20 cpufreq driver" that was fully
>>> reviewed, but seems not applied yet. Let me know if you prefer to re-spin
>>> the [0], including this patch into the series.
>>>
>>> [0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=45321
>>
>> This is already picked by Rafael and is sitting in pm/bleeding-edge
>> branch. Should get merged into linux-next in a day or two.
>>
>>>  drivers/cpufreq/tegra20-cpufreq.c | 16 +++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c 
>>> b/drivers/cpufreq/tegra20-cpufreq.c
>>> index 05f57dcd5215..ca5229265b60 100644
>>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>>> @@ -176,6 +176,14 @@ static int tegra20_cpufreq_probe(struct 
>>> platform_device *pdev)
>>> goto put_pll_x;
>>> }
>>>  
>>> +   err = clk_prepare_enable(cpufreq->pll_x_clk);
>>> +   if (err)
>>> +   goto put_pll_p;
>>> +
>>> +   err = clk_prepare_enable(cpufreq->pll_p_clk);
>>> +   if (err)
>>> +   goto disable_pll_x;
>>> +
>>> cpufreq->dev = >dev;
>>> cpufreq->driver.get = cpufreq_generic_get;
>>> cpufreq->driver.attr = cpufreq_generic_attr;
>>> @@ -192,12 +200,16 @@ static int tegra20_cpufreq_probe(struct 
>>> platform_device *pdev)
>>>  
>>> err = cpufreq_register_driver(>driver);
>>> if (err)
>>> -   goto put_pll_p;
>>> +   goto disable_pll_p;
>>>  
>>> platform_set_drvdata(pdev, cpufreq);
>>>  
>>> return 0;
>>>  
>>> +disable_pll_p:
>>> +   clk_disable_unprepare(cpufreq->pll_p_clk);
>>> +disable_pll_x:
>>> +   clk_disable_unprepare(cpufreq->pll_x_clk);
>>>  put_pll_p:
>>> clk_put(cpufreq->pll_p_clk);
>>>  put_pll_x:
>>> @@ -214,6 +226,8 @@ static int tegra20_cpufreq_remove(struct 
>>> platform_device *pdev)
>>>  
>>> cpufreq_unregister_driver(>driver);
>>>  
>>> +   clk_disable_unprepare(cpufreq->pll_p_clk);
>>> +   clk_disable_unprepare(cpufreq->pll_x_clk);
>>> clk_put(cpufreq->pll_p_clk);
>>> clk_put(cpufreq->pll_x_clk);
>>> clk_put(cpufreq->cpu_clk);
>>
>> Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
>>
> 
> Please hold on this patch, seems I interpreted the cpufreq driver logic
> incorrectly and it is probably fine as it is. I'll re-check later today.
> 

I've re-checked the driver and it is fine, PLL_X disabling is expected on a
switching parent clock to PLL_P as for the target frequency. This patch isn't
needed, please scratch it.

It occurred to me that the driver could be improved further a tad while I was
performing the re-check, hence likely there will be couple more patches from me.

Re: [PATCH v1 4/5] ARM: tegra: Don't apply CPU erratas in insecure mode

[Dmitry Osipenko]

On 20.05.2018 13:15, Dmitry Osipenko wrote:
> CPU isn't allowed to touch secure registers while running under secure
> monitor. Hence skip applying CPU erratas in the reset handler if Trusted
> Foundations firmware presents.
> 
> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
> ---
>  arch/arm/mach-tegra/reset-handler.S | 27 +++
>  arch/arm/mach-tegra/reset.c |  3 +++
>  arch/arm/mach-tegra/reset.h |  4 +++-
>  3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/reset-handler.S 
> b/arch/arm/mach-tegra/reset-handler.S
> index 805f306fa6f7..d84c74a95806 100644
> --- a/arch/arm/mach-tegra/reset-handler.S
> +++ b/arch/arm/mach-tegra/reset-handler.S
> @@ -121,6 +121,12 @@ ENTRY(__tegra_cpu_reset_handler)
>   cpsid   aif, 0x13   @ SVC mode, interrupts disabled
>  
>   tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
> +
> + adr r12, __tegra_cpu_reset_handler_data
> + ldr r0, [r12, #RESET_DATA(TF_PRESENT)]
> + cmp r0, #0
> + bne after_errata
> +
>  #ifdef CONFIG_ARCH_TEGRA_2x_SOC
>  t20_check:
>   cmp r6, #TEGRA20
> @@ -155,7 +161,6 @@ after_errata:
>   and r10, r10, #0x3  @ R10 = CPU number
>   mov r11, #1
>   mov r11, r11, lsl r10   @ R11 = CPU mask
> - adr r12, __tegra_cpu_reset_handler_data
>  
>  #ifdef CONFIG_SMP
>   /* Does the OS know about this CPU? */
> @@ -169,10 +174,9 @@ after_errata:
>   cmp r6, #TEGRA20
>   bne 1f
>   /* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
> - mov32   r5, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
>   mov r0, #CPU_NOT_RESETTABLE
>   cmp r10, #0
> - strneb  r0, [r5, #__tegra20_cpu1_resettable_status_offset]
> + strneb  r0, [r12, #RESET_DATA(RESETTABLE_STATUS)]
>  1:
>  #endif
>  
> @@ -278,13 +282,20 @@ ENDPROC(__tegra_cpu_reset_handler)
>   .type   __tegra_cpu_reset_handler_data, %object
>   .globl  __tegra_cpu_reset_handler_data
>  __tegra_cpu_reset_handler_data:
> - .rept   TEGRA_RESET_DATA_SIZE
> - .long   0
> - .endr
> + .long   0   /* TEGRA_RESET_MASK_PRESENT */
> + .long   0   /* TEGRA_RESET_MASK_LP1 */
> + .long   0   /* TEGRA_RESET_MASK_LP2 */
> + .long   0   /* TEGRA_RESET_STARTUP_SECONDARY */
> + .long   0   /* TEGRA_RESET_STARTUP_LP2 */
> + .long   0   /* TEGRA_RESET_STARTUP_LP1 */
> +
>   .globl  __tegra20_cpu1_resettable_status_offset
>   .equ__tegra20_cpu1_resettable_status_offset, \
>   . - __tegra_cpu_reset_handler_start
> - .byte   0
> - .align L1_CACHE_SHIFT
> + .long   0   /* TEGRA_RESET_RESETTABLE_STATUS */
>  
> + .globl  __tegra_tf_present
> + .equ__tegra_tf_present, . - __tegra_cpu_reset_handler_start

I've noticed that __tegra_tf_present shouldn't belong to this patch, I've missed
to remove it while was rebasing.

Also, it occurred to me that it will be much better to remove the whole array
__tegra_cpu_reset_handler_data definition in the asm and get back to the
original ".rept   TEGRA_RESET_DATA_SIZE" instead. That will make this part of
code much nicer, I'll change that in v2.

Russell / Thierry, please give you acks-reviews where appropriate and let me
know if I should change anything else in v2, thanks.

> + .long   0   /* TEGRA_RESET_TF_PRESENT */
> + .align L1_CACHE_SHIFT
>  ENTRY(__tegra_cpu_reset_handler_end)
> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
> index dc558892753c..b02ae7699842 100644
> --- a/arch/arm/mach-tegra/reset.c
> +++ b/arch/arm/mach-tegra/reset.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "iomap.h"
>  #include "irammap.h"
> @@ -89,6 +90,8 @@ static void __init tegra_cpu_reset_handler_enable(void)
>  
>  void __init tegra_cpu_reset_handler_init(void)
>  {
> + __tegra_cpu_reset_handler_data[TEGRA_RESET_TF_PRESENT] =
> + trusted_foundations_registered();
>  
>  #ifdef CONFIG_SMP
>   __tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
> diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
> index 9c479c7925b8..0d9ddc022ece 100644
> --- a/arch/arm/mach-tegra/reset.h
> +++ b/arch/arm/mach-tegra/reset.h
> @@ -25,7 +25,9 @@
>  #define TEGRA_RESET_STARTUP_SECONDARY3
>  #define TEGRA_RESET_STARTUP_LP2  4
>  #define TEGRA_RESET_STARTUP_LP1  5
> -#define TEGRA_RESET_DATA_SIZE6
> +#define TEGRA_RESET_RESETTABLE_STATUS6
> +#define TEGRA_RESET_TF_PRESENT   7
> +#define TEGRA_RESET_DATA_SIZE8
>  
>  #ifndef __ASSEMBLY__
>  
> 

[PATCH v1] gpu: host1x: Utilize IOMMU mapping for firewall-copied buffers

[Dmitry Osipenko]

Map firewall-copied buffers into Host1x's IOVA space, otherwise Host1x
CDMA can't access the command buffers and all submitted jobs fail if IOMMU
and Host1x firewall are enabled in the kernels config.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/gpu/host1x/job.c | 58 +++-
 include/linux/host1x.h   |  4 ++-
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index e2f4a4d93d20..57384a5b5059 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -449,10 +449,13 @@ static int validate(struct host1x_firewall *fw, struct 
host1x_job_gather *g)
 
 static inline int copy_gathers(struct host1x_job *job, struct device *dev)
 {
+   struct host1x *host = dev_get_drvdata(job->channel->dev->parent);
struct host1x_firewall fw;
+   dma_addr_t dma_addr;
size_t size = 0;
size_t offset = 0;
unsigned int i;
+   int err;
 
fw.job = job;
fw.dev = dev;
@@ -466,23 +469,55 @@ static inline int copy_gathers(struct host1x_job *job, 
struct device *dev)
size += g->words * sizeof(u32);
}
 
+   if (host->domain)
+   size = iova_align(>iova, size);
+
/*
 * Try a non-blocking allocation from a higher priority pools first,
 * as awaiting for the allocation here is a major performance hit.
 */
-   job->gather_copy_mapped = dma_alloc_wc(dev, size, >gather_copy,
-  GFP_NOWAIT);
+   job->gather_copy_mapped = dma_alloc_wc(dev, size,
+  >gather_copy_phys,
+  GFP_NOWAIT);
 
/* the higher priority allocation failed, try the generic-blocking */
if (!job->gather_copy_mapped)
job->gather_copy_mapped = dma_alloc_wc(dev, size,
-  >gather_copy,
+  >gather_copy_phys,
   GFP_KERNEL);
if (!job->gather_copy_mapped)
return -ENOMEM;
 
job->gather_copy_size = size;
 
+   if (host->domain) {
+   unsigned long shift;
+
+   shift = iova_shift(>iova);
+   job->gather_copy_iova_alloc = alloc_iova(
+   >iova, size >> shift,
+   host->iova_end >> shift, true);
+   if (!job->gather_copy_iova_alloc)
+   return -ENOMEM;
+
+   job->gather_copy_iova = iova_dma_addr(
+   >iova, job->gather_copy_iova_alloc);
+
+   err = iommu_map(host->domain,
+   job->gather_copy_iova,
+   job->gather_copy_phys,
+   size, IOMMU_READ);
+   if (err) {
+   __free_iova(>iova, job->gather_copy_iova_alloc);
+   job->gather_copy_iova_alloc = NULL;
+   return err;
+   }
+
+   dma_addr = job->gather_copy_iova;
+   } else {
+   dma_addr = job->gather_copy_phys;
+   }
+
for (i = 0; i < job->num_gathers; i++) {
struct host1x_job_gather *g = >gathers[i];
void *gather;
@@ -494,7 +529,7 @@ static inline int copy_gathers(struct host1x_job *job, 
struct device *dev)
host1x_bo_munmap(g->bo, gather);
 
/* Store the location in the buffer */
-   g->base = job->gather_copy;
+   g->base = dma_addr;
g->offset = offset;
 
/* Validate the job */
@@ -582,9 +617,20 @@ void host1x_job_unpin(struct host1x_job *job)
 
job->num_unpins = 0;
 
-   if (job->gather_copy_size)
+   if (job->gather_copy_size) {
dma_free_wc(job->channel->dev, job->gather_copy_size,
-   job->gather_copy_mapped, job->gather_copy);
+   job->gather_copy_mapped, job->gather_copy_phys);
+
+   if (job->gather_copy_iova_alloc) {
+   iommu_unmap(host->domain,
+   job->gather_copy_iova,
+   job->gather_copy_size);
+
+   __free_iova(>iova, job->gather_copy_iova_alloc);
+
+   job->gather_copy_iova_alloc = NULL;
+   }
+   }
 }
 EXPORT_SYMBOL(host1x_job_unpin);
 
diff --git a/include/linux/host1x.h b/include/linux/host1x.h
index 57d26406bdfd..536a678f81d4 100644
--- a/include/linux/host1x.h
+++ b/include/linu

Re: [PATCH v2 0/5] Tegra20 External Memory Controller driver

[Dmitry Osipenko]

On 06.06.2018 13:45, Thierry Reding wrote:
> On Mon, Jun 04, 2018 at 01:36:49AM +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> Couple years ago the Tegra20 EMC driver was removed from the kernel
>> due to incompatible changes in the Tegra's clock driver. This patchset
>> introduces a modernized EMC driver. Currently the sole purpose of the
>> driver is to initialize DRAM frequency to maximum rate during of the
>> kernels boot-up. Later we may consider implementing dynamic memory
>> frequency scaling, utilizing functionality provided by this driver.
>>
>> Changelog:
>>
>> v2:
>>  - Minor code cleanups like consistent use of writel_relaxed instead
>>of non-relaxed version, reworded error messages, etc.
>>
>>  - Factored out use_pllm_ud bit checking into a standalone patch for
>>consistency.
>>
>> Dmitry Osipenko (5):
>>   dt: bindings: tegra20-emc: Document interrupt property
>>   ARM: dts: tegra20: Add interrupt to External Memory Controller
>>   clk: tegra20: Turn EMC clock gate into divider
>>   clk: tegra20: Check whether direct PLLM sourcing is turned off for EMC
>>   memory: tegra: Introduce Tegra20 EMC driver
> 
> I took a brief look and didn't spot any dependencies between the clk and
> memory patches. Is it correct that these can be applied separately?

Yes, it is correct.

Re: [PATCH v2 5/5] memory: tegra: Introduce Tegra20 EMC driver

[Dmitry Osipenko]

On 06.06.2018 14:02, Thierry Reding wrote:
> On Mon, Jun 04, 2018 at 01:36:54AM +0300, Dmitry Osipenko wrote:
>> Introduce driver for the External Memory Controller (EMC) found on Tegra20
>> chips, which controls the external DRAM on the board. The purpose of this
>> driver is to program memory timing for external memory on the EMC clock
>> rate change.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/memory/tegra/Kconfig   |  10 +
>>  drivers/memory/tegra/Makefile  |   1 +
>>  drivers/memory/tegra/tegra20-emc.c | 586 +
>>  3 files changed, 597 insertions(+)
>>  create mode 100644 drivers/memory/tegra/tegra20-emc.c
>>
>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>> index 6d74e499e18d..34e0b70f5c5f 100644
>> --- a/drivers/memory/tegra/Kconfig
>> +++ b/drivers/memory/tegra/Kconfig
>> @@ -6,6 +6,16 @@ config TEGRA_MC
>>This driver supports the Memory Controller (MC) hardware found on
>>NVIDIA Tegra SoCs.
>>  
>> +config TEGRA20_EMC
>> +bool "NVIDIA Tegra20 External Memory Controller driver"
>> +default y
>> +depends on ARCH_TEGRA_2x_SOC
>> +help
>> +  This driver is for the External Memory Controller (EMC) found on
>> +  Tegra20 chips. The EMC controls the external DRAM on the board.
>> +  This driver is required to change memory timings / clock rate for
>> +  external memory.
>> +
>>  config TEGRA124_EMC
>>  bool "NVIDIA Tegra124 External Memory Controller driver"
>>  default y
>> diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile
>> index 94ab16ba075b..3971a6b7c487 100644
>> --- a/drivers/memory/tegra/Makefile
>> +++ b/drivers/memory/tegra/Makefile
>> @@ -10,5 +10,6 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o
>>  
>>  obj-$(CONFIG_TEGRA_MC) += tegra-mc.o
>>  
>> +obj-$(CONFIG_TEGRA20_EMC)  += tegra20-emc.o
>>  obj-$(CONFIG_TEGRA124_EMC) += tegra124-emc.o
>>  obj-$(CONFIG_ARCH_TEGRA_186_SOC) += tegra186.o
>> diff --git a/drivers/memory/tegra/tegra20-emc.c 
>> b/drivers/memory/tegra/tegra20-emc.c
>> new file mode 100644
>> index ..26a18b5e7941
>> --- /dev/null
>> +++ b/drivers/memory/tegra/tegra20-emc.c
>> @@ -0,0 +1,586 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Tegra20 External Memory Controller driver
>> + *
>> + * Author: Dmitry Osipenko 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +#define EMC_INTSTATUS   0x000
>> +#define EMC_INTMASK 0x004
>> +#define EMC_TIMING_CONTROL  0x028
>> +#define EMC_RC  0x02c
>> +#define EMC_RFC 0x030
>> +#define EMC_RAS 0x034
>> +#define EMC_RP  0x038
>> +#define EMC_R2W 0x03c
>> +#define EMC_W2R 0x040
>> +#define EMC_R2P 0x044
>> +#define EMC_W2P 0x048
>> +#define EMC_RD_RCD  0x04c
>> +#define EMC_WR_RCD  0x050
>> +#define EMC_RRD 0x054
>> +#define EMC_REXT0x058
>> +#define EMC_WDV 0x05c
>> +#define EMC_QUSE0x060
>> +#define EMC_QRST0x064
>> +#define EMC_QSAFE   0x068
>> +#define EMC_RDV 0x06c
>> +#define EMC_REFRESH 0x070
>> +#define EMC_BURST_REFRESH_NUM   0x074
>> +#define EMC_PDEX2WR 0x078
>> +#define EMC_PDEX2RD 0x07c
>> +#define EMC_PCHG2PDEN   0x080
>> +#define EMC_ACT2PDEN0x084
>> +#define EMC_AR2PDEN 0x088
>> +#define EMC_RW2PDEN 0x08c
>> +#define EMC_TXSR0x090
>> +#define EMC_TCKE0x094
>> +#define EMC_TFAW

Re: [PATCH v4 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On Monday, 11 June 2018 23:52:22 MSK Stefan Agner wrote:
> Add support for the NAND flash controller found on NVIDIA
> Tegra 2 SoCs. This implementation does not make use of the
> command queue feature. Regular operations/data transfers are
> done in PIO mode. Page read/writes with hardware ECC make
> use of the DMA for data transfer.
> 
> Signed-off-by: Lucas Stach 
> Signed-off-by: Stefan Agner 
> ---
>  MAINTAINERS   |7 +
>  drivers/mtd/nand/raw/Kconfig  |6 +
>  drivers/mtd/nand/raw/Makefile |1 +
>  drivers/mtd/nand/raw/tegra_nand.c | 1248 +
>  4 files changed, 1262 insertions(+)
>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 58b9861ccf99..c2e5571c85d4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13844,6 +13844,13 @@ M:   Laxman Dewangan 
>  S:   Supported
>  F:   drivers/input/keyboard/tegra-kbc.c
> 
> +TEGRA NAND DRIVER
> +M:   Stefan Agner 
> +M:   Lucas Stach 
> +S:   Maintained
> +F:   Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> +F:   drivers/mtd/nand/raw/tegra_nand.c
> +
>  TEGRA PWM DRIVER
>  M:   Thierry Reding 
>  S:   Supported
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index 19a2b283fbbe..e9093f52371e 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -534,4 +534,10 @@ config MTD_NAND_MTK
> Enables support for NAND controller on MTK SoCs.
> This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> 
> +config MTD_NAND_TEGRA
> + tristate "Support for NAND controller on NVIDIA Tegra"
> + depends on ARCH_TEGRA || COMPILE_TEST
> + help
> +   Enables support for NAND flash controller on NVIDIA Tegra SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> index 165b7ef9e9a1..d5a5f9832b88 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)  += 
hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)  += brcmnand/
>  obj-$(CONFIG_MTD_NAND_QCOM)  += qcom_nandc.o
>  obj-$(CONFIG_MTD_NAND_MTK)   += mtk_ecc.o mtk_nand.o
> +obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o
> 
>  nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
>  nand-objs += nand_amd.o
> diff --git a/drivers/mtd/nand/raw/tegra_nand.c
> b/drivers/mtd/nand/raw/tegra_nand.c new file mode 100644
> index ..dd23a5eb6af3
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -0,0 +1,1248 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Stefan Agner 
> + * Copyright (C) 2014-2015 Lucas Stach 
> + * Copyright (C) 2012 Avionic Design GmbH
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define COMMAND  0x00
> +#define   COMMAND_GO BIT(31)
> +#define   COMMAND_CLEBIT(30)
> +#define   COMMAND_ALEBIT(29)
> +#define   COMMAND_PIOBIT(28)
> +#define   COMMAND_TX BIT(27)
> +#define   COMMAND_RX BIT(26)
> +#define   COMMAND_SEC_CMDBIT(25)
> +#define   COMMAND_AFT_DATBIT(24)
> +#define   COMMAND_TRANS_SIZE(x)  (((x - 1) & 0xf) << 20)
> +#define   COMMAND_A_VALIDBIT(19)
> +#define   COMMAND_B_VALIDBIT(18)
> +#define   COMMAND_RD_STATUS_CHK  BIT(17)
> +#define   COMMAND_RBSY_CHK   BIT(16)
> +#define   COMMAND_CE(x)  BIT(8 + ((x) & 0x7))
> +#define   COMMAND_CLE_SIZE(x)(((x - 1) & 0x3) << 4)
> +#define   COMMAND_ALE_SIZE(x)(((x - 1) & 0xf) << 0)
> +
> +#define STATUS   0x04
> +
> +#define ISR  0x08
> +#define   ISR_CORRFAIL_ERR   BIT(24)
> +#define   ISR_UNDBIT(7)
> +#define   ISR_OVRBIT(6)
> +#define   ISR_CMD_DONE   BIT(5)
> +#define   ISR_ECC_ERRBIT(4)
> +
> +#define IER  0x0c
> +#define   IER_ERR_TRIG_VAL(x)(((x) & 0xf) << 16)
> +#define   IER_UNDBIT(7)
> +#define   IER_OVRBIT(6)
> +#define   IER_CMD_DONE   BIT(5)
> +#define   IER_ECC_ERRBIT(4)
> +#define   IER_GIEBIT(0)
> +
> +#define CONFIG  

Re: [PATCH v4 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On Monday, 11 June 2018 23:52:22 MSK Stefan Agner wrote:
> Add support for the NAND flash controller found on NVIDIA
> Tegra 2 SoCs. This implementation does not make use of the
> command queue feature. Regular operations/data transfers are
> done in PIO mode. Page read/writes with hardware ECC make
> use of the DMA for data transfer.

Are "regular operations/data transfers" == "raw read/writes"? Seems you've 
fixed DMA now and hence should adjust the commit message if that's the case.

> 
> Signed-off-by: Lucas Stach 
> Signed-off-by: Stefan Agner 
> ---

Otherwise looks good to me:

Reviewed-by: Dmitry Osipenko 


There couple nit comments about newlines below, maybe you could clean up them 
if there will be another revision.

>  MAINTAINERS   |7 +
>  drivers/mtd/nand/raw/Kconfig  |6 +
>  drivers/mtd/nand/raw/Makefile |1 +
>  drivers/mtd/nand/raw/tegra_nand.c | 1248 +
>  4 files changed, 1262 insertions(+)
>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 58b9861ccf99..c2e5571c85d4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13844,6 +13844,13 @@ M:   Laxman Dewangan 
>  S:   Supported
>  F:   drivers/input/keyboard/tegra-kbc.c
> 
> +TEGRA NAND DRIVER
> +M:   Stefan Agner 
> +M:   Lucas Stach 
> +S:   Maintained
> +F:   Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> +F:   drivers/mtd/nand/raw/tegra_nand.c
> +
>  TEGRA PWM DRIVER
>  M:   Thierry Reding 
>  S:   Supported
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index 19a2b283fbbe..e9093f52371e 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -534,4 +534,10 @@ config MTD_NAND_MTK
> Enables support for NAND controller on MTK SoCs.
> This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> 
> +config MTD_NAND_TEGRA
> + tristate "Support for NAND controller on NVIDIA Tegra"
> + depends on ARCH_TEGRA || COMPILE_TEST
> + help
> +   Enables support for NAND flash controller on NVIDIA Tegra SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> index 165b7ef9e9a1..d5a5f9832b88 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)  += 
hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)  += brcmnand/
>  obj-$(CONFIG_MTD_NAND_QCOM)  += qcom_nandc.o
>  obj-$(CONFIG_MTD_NAND_MTK)   += mtk_ecc.o mtk_nand.o
> +obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o
> 
>  nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
>  nand-objs += nand_amd.o
> diff --git a/drivers/mtd/nand/raw/tegra_nand.c
> b/drivers/mtd/nand/raw/tegra_nand.c new file mode 100644
> index ..dd23a5eb6af3
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -0,0 +1,1248 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Stefan Agner 
> + * Copyright (C) 2014-2015 Lucas Stach 
> + * Copyright (C) 2012 Avionic Design GmbH
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define COMMAND  0x00
> +#define   COMMAND_GO BIT(31)
> +#define   COMMAND_CLEBIT(30)
> +#define   COMMAND_ALEBIT(29)
> +#define   COMMAND_PIOBIT(28)
> +#define   COMMAND_TX BIT(27)
> +#define   COMMAND_RX BIT(26)
> +#define   COMMAND_SEC_CMDBIT(25)
> +#define   COMMAND_AFT_DATBIT(24)
> +#define   COMMAND_TRANS_SIZE(x)  (((x - 1) & 0xf) << 20)
> +#define   COMMAND_A_VALIDBIT(19)
> +#define   COMMAND_B_VALIDBIT(18)
> +#define   COMMAND_RD_STATUS_CHK  BIT(17)
> +#define   COMMAND_RBSY_CHK   BIT(16)
> +#define   COMMAND_CE(x)  BIT(8 + ((x) & 0x7))
> +#define   COMMAND_CLE_SIZE(x)(((x - 1) & 0x3) << 4)
> +#define   COMMAND_ALE_SIZE(x)(((x - 1) & 0xf) << 0)
> +
> +#define STATUS   0x04
> +
> +#define ISR  0x08
> +#define   ISR_CORRFAIL_ERR 

Re: [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On Sunday, 10 June 2018 14:09:24 MSK Stefan Agner wrote:
> On 09.06.2018 14:21, Dmitry Osipenko wrote:
> > On Saturday, 9 June 2018 00:51:01 MSK Stefan Agner wrote:
> >> On 01.06.2018 11:20, Dmitry Osipenko wrote:
> >> > On 01.06.2018 01:16, Stefan Agner wrote:
> >> >> Add support for the NAND flash controller found on NVIDIA
> >> >> Tegra 2 SoCs. This implementation does not make use of the
> >> >> command queue feature. Regular operations/data transfers are
> >> >> done in PIO mode. Page read/writes with hardware ECC make
> >> >> use of the DMA for data transfer.
> >> >> 
> >> >> Signed-off-by: Lucas Stach 
> >> >> Signed-off-by: Stefan Agner 
> >> >> ---
> >> >> 
> >> >>  MAINTAINERS   |7 +
> >> >>  drivers/mtd/nand/raw/Kconfig  |6 +
> >> >>  drivers/mtd/nand/raw/Makefile |1 +
> >> >>  drivers/mtd/nand/raw/tegra_nand.c | 1143
> >> >>  +
> >> >>  4 files changed, 1157 insertions(+)
> >> >>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
> >> >> 
> >> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> >> index 58b9861ccf99..c2e5571c85d4 100644
> >> >> --- a/MAINTAINERS
> >> >> +++ b/MAINTAINERS
> >> >> @@ -13844,6 +13844,13 @@ M: Laxman Dewangan 
> >> >> 
> >> >>  S: Supported
> >> >>  F: drivers/input/keyboard/tegra-kbc.c
> >> >> 
> >> >> +TEGRA NAND DRIVER
> >> >> +M: Stefan Agner 
> >> >> +M: Lucas Stach 
> >> >> +S: Maintained
> >> >> +F: Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> >> +F: drivers/mtd/nand/raw/tegra_nand.c
> >> >> +
> >> >> 
> >> >>  TEGRA PWM DRIVER
> >> >>  M: Thierry Reding 
> >> >>  S: Supported
> >> >> 
> >> >> diff --git a/drivers/mtd/nand/raw/Kconfig
> >> >> b/drivers/mtd/nand/raw/Kconfig
> >> >> index 19a2b283fbbe..e9093f52371e 100644
> >> >> --- a/drivers/mtd/nand/raw/Kconfig
> >> >> +++ b/drivers/mtd/nand/raw/Kconfig
> >> >> @@ -534,4 +534,10 @@ config MTD_NAND_MTK
> >> >> 
> >> >>   Enables support for NAND controller on MTK SoCs.
> >> >>   This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> >> >> 
> >> >> +config MTD_NAND_TEGRA
> >> >> +   tristate "Support for NAND controller on NVIDIA Tegra"
> >> >> +   depends on ARCH_TEGRA || COMPILE_TEST
> >> >> +   help
> >> >> + Enables support for NAND flash controller on NVIDIA Tegra SoC.
> >> >> +
> >> >> 
> >> >>  endif # MTD_NAND
> >> >> 
> >> >> diff --git a/drivers/mtd/nand/raw/Makefile
> >> >> b/drivers/mtd/nand/raw/Makefile
> >> >> index 165b7ef9e9a1..d5a5f9832b88 100644
> >> >> --- a/drivers/mtd/nand/raw/Makefile
> >> >> +++ b/drivers/mtd/nand/raw/Makefile
> >> >> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)+=
> >> >> hisi504_nand.o
> >> >> 
> >> >>  obj-$(CONFIG_MTD_NAND_BRCMNAND)+= brcmnand/
> >> >>  obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o
> >> >>  obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o
> >> >> 
> >> >> +obj-$(CONFIG_MTD_NAND_TEGRA)   += tegra_nand.o
> >> >> 
> >> >>  nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
> >> >>  nand-objs += nand_amd.o
> >> >> 
> >> >> diff --git a/drivers/mtd/nand/raw/tegra_nand.c
> >> >> b/drivers/mtd/nand/raw/tegra_nand.c new file mode 100644
> >> >> index ..e9664f2938a3
> >> >> --- /dev/null
> >> >> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> >> >> @@ -0,0 +1,1143 @@
> >> >> +// SPDX-License-Identifier: GPL-2.0
> >> >> +/*
> >> >> + * Copyright (C) 2018 Stefan Agner 
> >> >> + * Copyright (C) 2014-2015 Lucas Stach 
> >> >> + * Copyright (C) 2012 Avionic Design GmbH
> >> >> + */
> &

Re: [PATCH v4 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On Monday, 11 June 2018 23:52:22 MSK Stefan Agner wrote:
> Add support for the NAND flash controller found on NVIDIA
> Tegra 2 SoCs. This implementation does not make use of the
> command queue feature. Regular operations/data transfers are
> done in PIO mode. Page read/writes with hardware ECC make
> use of the DMA for data transfer.
> 
> Signed-off-by: Lucas Stach 
> Signed-off-by: Stefan Agner 
> ---
>  MAINTAINERS   |7 +
>  drivers/mtd/nand/raw/Kconfig  |6 +
>  drivers/mtd/nand/raw/Makefile |1 +
>  drivers/mtd/nand/raw/tegra_nand.c | 1248 +
>  4 files changed, 1262 insertions(+)
>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 58b9861ccf99..c2e5571c85d4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13844,6 +13844,13 @@ M:   Laxman Dewangan 
>  S:   Supported
>  F:   drivers/input/keyboard/tegra-kbc.c
> 
> +TEGRA NAND DRIVER
> +M:   Stefan Agner 
> +M:   Lucas Stach 
> +S:   Maintained
> +F:   Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> +F:   drivers/mtd/nand/raw/tegra_nand.c
> +
>  TEGRA PWM DRIVER
>  M:   Thierry Reding 
>  S:   Supported
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index 19a2b283fbbe..e9093f52371e 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -534,4 +534,10 @@ config MTD_NAND_MTK
> Enables support for NAND controller on MTK SoCs.
> This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> 
> +config MTD_NAND_TEGRA
> + tristate "Support for NAND controller on NVIDIA Tegra"
> + depends on ARCH_TEGRA || COMPILE_TEST
> + help
> +   Enables support for NAND flash controller on NVIDIA Tegra SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> index 165b7ef9e9a1..d5a5f9832b88 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)  += 
hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)  += brcmnand/
>  obj-$(CONFIG_MTD_NAND_QCOM)  += qcom_nandc.o
>  obj-$(CONFIG_MTD_NAND_MTK)   += mtk_ecc.o mtk_nand.o
> +obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o
> 
>  nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
>  nand-objs += nand_amd.o
> diff --git a/drivers/mtd/nand/raw/tegra_nand.c
> b/drivers/mtd/nand/raw/tegra_nand.c new file mode 100644
> index ..dd23a5eb6af3
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -0,0 +1,1248 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Stefan Agner 
> + * Copyright (C) 2014-2015 Lucas Stach 
> + * Copyright (C) 2012 Avionic Design GmbH
> + */
> +
> +#include 
> +#include 
> +#include 

The "delay.h" looks obsolete now, maybe you could drop it.

Re: [PATCH v1] clk: tegra: emc: Avoid out-of-bounds bug

[Dmitry Osipenko]

On Tuesday, 12 June 2018 10:46:03 MSK Stephen Boyd wrote:
> Quoting Dmitry Osipenko (2018-06-05 05:12:32)
> 
> > Apparently there was an attempt to avoid out-of-bounds accesses when there
> > is only one memory timing available, but there is a typo in the code that
> > neglects that attempt.
> > 
> > Signed-off-by: Dmitry Osipenko 
> 
> Fixes: tag?

This is kinda a minor fix for an issue that shouldn't happen because today 
AFAIK nothing on Tegra imposes the max/min clock rate constraints. There is no 
need to backport this patch.

[PATCH v3 4/8] ARM: dts: tegra20: Add interrupt entry to External Memory Controller

[Dmitry Osipenko]

Add interrupt entry into the EMC DT node.

Signed-off-by: Dmitry Osipenko 
Acked-by: Peter De Schrijver 
---
 arch/arm/boot/dts/tegra20.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 983dd5c14794..3cd3cb28cfd9 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -609,6 +609,7 @@
memory-controller@7000f400 {
compatible = "nvidia,tegra20-emc";
reg = <0x7000f400 0x200>;
+   interrupts = ;
#address-cells = <1>;
#size-cells = <0>;
};
-- 
2.17.1

[PATCH v3 2/8] dt: bindings: tegra20-emc: Document clock property

[Dmitry Osipenko]

Embedded memory controller has a corresponding clock, document the clock
property.

Signed-off-by: Dmitry Osipenko 
---
 .../devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt| 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt 
b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt
index a6fe401d0d48..add95367640b 100644
--- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt
@@ -11,6 +11,7 @@ Properties:
   set of tables can be present and said tables will be used
   irrespective of ram-code configuration.
 - interrupts : Should contain EMC General interrupt.
+- clocks : Should contain EMC clock.
 
 Child device nodes describe the memory settings for different configurations 
and clock rates.
 
@@ -22,6 +23,7 @@ Example:
compatible = "nvidia,tegra20-emc";
reg = <0x7000f4000 0x200>;
interrupts = <0 78 0x04>;
+   clocks = <_car TEGRA20_CLK_EMC>;
}
 
 
-- 
2.17.1

[PATCH v3 1/8] dt: bindings: tegra20-emc: Document interrupt property

[Dmitry Osipenko]

EMC has a dedicated interrupt that is used to notify about completion of
HW operations. Document the interrupt property.

Signed-off-by: Dmitry Osipenko 
Acked-by: Rob Herring 
Acked-by: Peter De Schrijver 
---
 .../devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt| 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt 
b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt
index 4c33b29dc660..a6fe401d0d48 100644
--- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt
@@ -10,6 +10,7 @@ Properties:
   and chosen using the ramcode board selector. If omitted, only one
   set of tables can be present and said tables will be used
   irrespective of ram-code configuration.
+- interrupts : Should contain EMC General interrupt.
 
 Child device nodes describe the memory settings for different configurations 
and clock rates.
 
@@ -20,6 +21,7 @@ Example:
#size-cells = < 0 >;
compatible = "nvidia,tegra20-emc";
reg = <0x7000f4000 0x200>;
+   interrupts = <0 78 0x04>;
}
 
 
-- 
2.17.1

[PATCH v3 8/8] memory: tegra: Introduce Tegra20 EMC driver

[Dmitry Osipenko]

Introduce driver for the External Memory Controller (EMC) found on Tegra20
chips, which controls the external DRAM on the board. The purpose of this
driver is to program memory timing for external memory on the EMC clock
rate change.

Signed-off-by: Dmitry Osipenko 
Acked-by: Peter De Schrijver 
---
 drivers/memory/tegra/Kconfig   |  10 +
 drivers/memory/tegra/Makefile  |   1 +
 drivers/memory/tegra/tegra20-emc.c | 574 +
 3 files changed, 585 insertions(+)
 create mode 100644 drivers/memory/tegra/tegra20-emc.c

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 6d74e499e18d..34e0b70f5c5f 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -6,6 +6,16 @@ config TEGRA_MC
  This driver supports the Memory Controller (MC) hardware found on
  NVIDIA Tegra SoCs.
 
+config TEGRA20_EMC
+   bool "NVIDIA Tegra20 External Memory Controller driver"
+   default y
+   depends on ARCH_TEGRA_2x_SOC
+   help
+ This driver is for the External Memory Controller (EMC) found on
+ Tegra20 chips. The EMC controls the external DRAM on the board.
+ This driver is required to change memory timings / clock rate for
+ external memory.
+
 config TEGRA124_EMC
bool "NVIDIA Tegra124 External Memory Controller driver"
default y
diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile
index 94ab16ba075b..3971a6b7c487 100644
--- a/drivers/memory/tegra/Makefile
+++ b/drivers/memory/tegra/Makefile
@@ -10,5 +10,6 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o
 
 obj-$(CONFIG_TEGRA_MC) += tegra-mc.o
 
+obj-$(CONFIG_TEGRA20_EMC)  += tegra20-emc.o
 obj-$(CONFIG_TEGRA124_EMC) += tegra124-emc.o
 obj-$(CONFIG_ARCH_TEGRA_186_SOC) += tegra186.o
diff --git a/drivers/memory/tegra/tegra20-emc.c 
b/drivers/memory/tegra/tegra20-emc.c
new file mode 100644
index ..b4a0066bdf4f
--- /dev/null
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -0,0 +1,574 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Tegra20 External Memory Controller driver
+ *
+ * Author: Dmitry Osipenko 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define EMC_INTSTATUS  0x000
+#define EMC_INTMASK0x004
+#define EMC_TIMING_CONTROL 0x028
+#define EMC_RC 0x02c
+#define EMC_RFC0x030
+#define EMC_RAS0x034
+#define EMC_RP 0x038
+#define EMC_R2W0x03c
+#define EMC_W2R0x040
+#define EMC_R2P0x044
+#define EMC_W2P0x048
+#define EMC_RD_RCD 0x04c
+#define EMC_WR_RCD 0x050
+#define EMC_RRD0x054
+#define EMC_REXT   0x058
+#define EMC_WDV0x05c
+#define EMC_QUSE   0x060
+#define EMC_QRST   0x064
+#define EMC_QSAFE  0x068
+#define EMC_RDV0x06c
+#define EMC_REFRESH0x070
+#define EMC_BURST_REFRESH_NUM  0x074
+#define EMC_PDEX2WR0x078
+#define EMC_PDEX2RD0x07c
+#define EMC_PCHG2PDEN  0x080
+#define EMC_ACT2PDEN   0x084
+#define EMC_AR2PDEN0x088
+#define EMC_RW2PDEN0x08c
+#define EMC_TXSR   0x090
+#define EMC_TCKE   0x094
+#define EMC_TFAW   0x098
+#define EMC_TRPAB  0x09c
+#define EMC_TCLKSTABLE 0x0a0
+#define EMC_TCLKSTOP   0x0a4
+#define EMC_TREFBW 0x0a8
+#define EMC_QUSE_EXTRA 0x0ac
+#define EMC_ODT_WRITE  0x0b0
+#define EMC_ODT_READ   0x0b4
+#define EMC_FBIO_CFG5  0x104
+#define EMC_FBIO_CFG6  0x114
+#define EMC_AUTO_CAL_INTERVAL  0x2a8
+#define EMC_CFG_2  0x2b8
+#define EMC_CFG_DIG_DLL0x2bc
+#define EMC_DLL_XFORM_DQS  0x2c0
+#define EMC_DLL_XFORM_QUSE 0x2c4
+#define EMC_ZCAL_REF_CNT   0x2e0
+#define EMC_ZCAL_WAIT_CNT  0x2e4
+#define EMC_CFG_CLKTRIM_0  0x2d0
+#defi

[PATCH v3 5/8] ARM: dts: tegra20: Add clock entry to External Memory Controller

[Dmitry Osipenko]

Add clock entry into the EMC DT node.

Signed-off-by: Dmitry Osipenko 
---
 arch/arm/boot/dts/tegra20.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 3cd3cb28cfd9..877e41fe87cf 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -610,6 +610,7 @@
compatible = "nvidia,tegra20-emc";
reg = <0x7000f400 0x200>;
interrupts = ;
+   clocks = <_car TEGRA20_CLK_EMC>;
#address-cells = <1>;
#size-cells = <0>;
};
-- 
2.17.1

[PATCH v3 3/8] dt: bindings: Move tegra20-emc binding to memory-controllers directory

[Dmitry Osipenko]

Move tegra20-emc binding to the memory-controllers directory for
consistency with the other Tegra memory bindings.

Signed-off-by: Dmitry Osipenko 
---
 .../{arm/tegra => memory-controllers}/nvidia,tegra20-emc.txt  | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/{arm/tegra => 
memory-controllers}/nvidia,tegra20-emc.txt (100%)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt
rename to 
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
-- 
2.17.1

[PATCH v3 6/8] clk: tegra20: Turn EMC clock gate into divider

[Dmitry Osipenko]

Kernel should never gate the EMC clock as it causes immediate lockup, so
removing clk-gate functionality doesn't affect anything. Turning EMC clk
gate into divider allows to implement glitch-less EMC scaling, avoiding
reparenting to a backup clock.

Signed-off-by: Dmitry Osipenko 
Acked-by: Peter De Schrijver 
---
 drivers/clk/tegra/clk-tegra20.c | 36 -
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index cc857d4d4a86..2bd35418716a 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -578,7 +578,6 @@ static struct tegra_clk tegra20_clks[tegra_clk_max] 
__initdata = {
[tegra_clk_afi] = { .dt_id = TEGRA20_CLK_AFI, .present = true },
[tegra_clk_fuse] = { .dt_id = TEGRA20_CLK_FUSE, .present = true },
[tegra_clk_kfuse] = { .dt_id = TEGRA20_CLK_KFUSE, .present = true },
-   [tegra_clk_emc] = { .dt_id = TEGRA20_CLK_EMC, .present = true },
 };
 
 static unsigned long tegra20_clk_measure_input_freq(void)
@@ -799,6 +798,31 @@ static struct tegra_periph_init_data 
tegra_periph_nodiv_clk_list[] = {
TEGRA_INIT_DATA_NODIV("disp2",  mux_pllpdc_clkm, CLK_SOURCE_DISP2, 30, 
2, 26,  0, TEGRA20_CLK_DISP2),
 };
 
+static void __init tegra20_emc_clk_init(void)
+{
+   struct clk *clk;
+
+   clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
+  ARRAY_SIZE(mux_pllmcp_clkm),
+  CLK_SET_RATE_NO_REPARENT,
+  clk_base + CLK_SOURCE_EMC,
+  30, 2, 0, _lock);
+
+   clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
+   _lock);
+   clks[TEGRA20_CLK_MC] = clk;
+
+   /*
+* Note that 'emc_mux' source and 'emc' rate shouldn't be changed at
+* the same time due to a HW bug, this won't happen because we're
+* defining 'emc_mux' and 'emc' as a distinct clocks.
+*/
+   clk = clk_register_divider(NULL, "emc", "emc_mux", CLK_IS_CRITICAL,
+  clk_base + CLK_SOURCE_EMC, 0, 7,
+  0, _lock);
+   clks[TEGRA20_CLK_EMC] = clk;
+}
+
 static void __init tegra20_periph_clk_init(void)
 {
struct tegra_periph_init_data *data;
@@ -812,15 +836,7 @@ static void __init tegra20_periph_clk_init(void)
clks[TEGRA20_CLK_AC97] = clk;
 
/* emc */
-   clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
-  ARRAY_SIZE(mux_pllmcp_clkm),
-  CLK_SET_RATE_NO_REPARENT,
-  clk_base + CLK_SOURCE_EMC,
-  30, 2, 0, _lock);
-
-   clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
-   _lock);
-   clks[TEGRA20_CLK_MC] = clk;
+   tegra20_emc_clk_init();
 
/* dsi */
clk = tegra_clk_register_periph_gate("dsi", "pll_d", 0, clk_base, 0,
-- 
2.17.1

[PATCH v3 0/8] Tegra20 External Memory Controller driver

[Dmitry Osipenko]

Hello,

Couple years ago the Tegra20 EMC driver was removed from the kernel
due to incompatible changes in the Tegra's clock driver. This patchset
introduces a modernized EMC driver. Currently the sole purpose of the
driver is to initialize DRAM frequency to maximum rate during of the
kernels boot-up. Later we may consider implementing dynamic memory
frequency scaling, utilizing functionality provided by this driver.

Changelog:

v3:
- Handle "nvidia,use-ram-code" DT property, its handling was missed
  in the previous versions.

- Honor "emc-tables" DT node naming which is explicitly specified
  in the DT binding, also was missed in the previous versions.

- Two new DT binding patches: one adds the EMC clock property,
  other relocates the binding doc file to the appropriate directory.
  One new patch that adds EMC clock property to the DTS file.

- Addressed v2 review comments from Thierry Reding. Driver does not
  preserve backwards compatibility with older device tree binding.

- The PLL_M and PLL_P clocks are kept internal to the driver because
  after some more considering I couldn't find a really good reason why
  these clocks should be in the device tree.

- Some minor cleanups and fixes in the drivers code.

v2:
- Minor code cleanups like consistent use of writel_relaxed instead
  of non-relaxed version, reworded error messages, etc.

- Factored out use_pllm_ud bit checking into a standalone patch for
  consistency.

Dmitry Osipenko (8):
  dt: bindings: tegra20-emc: Document interrupt property
  dt: bindings: tegra20-emc: Document clock property
  dt: bindings: Move tegra20-emc binding to memory-controllers directory
  ARM: dts: tegra20: Add interrupt entry to External Memory Controller
  ARM: dts: tegra20: Add clock entry to External Memory Controller
  clk: tegra20: Turn EMC clock gate into divider
  clk: tegra20: Check whether direct PLLM sourcing is turned off for EMC
  memory: tegra: Introduce Tegra20 EMC driver

 .../nvidia,tegra20-emc.txt|   4 +
 arch/arm/boot/dts/tegra20.dtsi|   2 +
 drivers/clk/tegra/clk-tegra20.c   |  46 +-
 drivers/memory/tegra/Kconfig  |  10 +
 drivers/memory/tegra/Makefile |   1 +
 drivers/memory/tegra/tegra20-emc.c| 574 ++
 6 files changed, 627 insertions(+), 10 deletions(-)
 rename Documentation/devicetree/bindings/{arm/tegra => 
memory-controllers}/nvidia,tegra20-emc.txt (95%)
 create mode 100644 drivers/memory/tegra/tegra20-emc.c

-- 
2.17.1

[PATCH v3 7/8] clk: tegra20: Check whether direct PLLM sourcing is turned off for EMC

[Dmitry Osipenko]

Ensure that direct PLLM sourcing is turned off for EMC as we don't support
that configuration in the clk driver.

Signed-off-by: Dmitry Osipenko 
Acked-by: Peter De Schrijver 
---
 drivers/clk/tegra/clk-tegra20.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 2bd35418716a..ca4eadb9520e 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -800,7 +800,9 @@ static struct tegra_periph_init_data 
tegra_periph_nodiv_clk_list[] = {
 
 static void __init tegra20_emc_clk_init(void)
 {
+   const u32 use_pllm_ud = BIT(29);
struct clk *clk;
+   u32 emc_reg;
 
clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
   ARRAY_SIZE(mux_pllmcp_clkm),
@@ -812,6 +814,14 @@ static void __init tegra20_emc_clk_init(void)
_lock);
clks[TEGRA20_CLK_MC] = clk;
 
+   /* un-divided pll_m_out0 is currently unsupported */
+   emc_reg = readl_relaxed(clk_base + CLK_SOURCE_EMC);
+   if (emc_reg & use_pllm_ud) {
+   pr_err("%s: un-divided PllM_out0 used as clock source\n",
+  __func__);
+   return;
+   }
+
/*
 * Note that 'emc_mux' source and 'emc' rate shouldn't be changed at
 * the same time due to a HW bug, this won't happen because we're
-- 
2.17.1

Re: [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On Sunday, 10 June 2018 18:32:02 MSK Boris Brezillon wrote:
> On Sun, 10 Jun 2018 18:00:06 +0300
> 
> Dmitry Osipenko  wrote:
> > > >> That seems a lot of work for a code path I do not intend to ever use
> > > >> :-)
> > > > 
> > > > Are you sure that resetting HW resets the timing and other registers
> > > > configuration? Reset implementation is HW-specific, like for example
> > > > in a
> > > > case of a video decoder the registers state is re-intialized on HW
> > > > reset,
> > > > but registers configuration is untouched in a case of resetting GPU.
> > > > I'd
> > > > suggest to check whether NAND controller resetting affects the HW
> > > > configuration.
> > > 
> > > It seems all registers are set back to their documented reset value:
> > > 
> > > [boot loader/ROM initialized values]
> > > [1.270253] tegra-nand 70008000.nand: Tegra NAND controller register
> > > dump
> > > [1.277051] tegra-nand 70008000.nand: COMMAND: 0x66880104
> > > [1.282457] tegra-nand 70008000.nand: STATUS: 0x0101
> > > [1.287763] tegra-nand 70008000.nand: ISR: 0x01000120
> > > [1.292818] tegra-nand 70008000.nand: IER: 0x
> > > [1.297863] tegra-nand 70008000.nand: CONFIG: 0x0084
> > > [1.303181] tegra-nand 70008000.nand: TIMING: 0x0504
> > > [1.308486] tegra-nand 70008000.nand: TIMING2: 0x0003
> > > [1.313897] tegra-nand 70008000.nand: CMD_REG1: 0x
> > > [1.319377] tegra-nand 70008000.nand: CMD_REG2: 0x0030
> > > [1.324868] tegra-nand 70008000.nand: ADDR_REG1: 0x0300
> > > [1.330435] tegra-nand 70008000.nand: ADDR_REG2: 0x
> > > [1.336011] tegra-nand 70008000.nand: DMA_MST_CTRL: 0x0414
> > > [1.341838] tegra-nand 70008000.nand: DMA_CFG_A: 0x0fff
> > > [1.347415] tegra-nand 70008000.nand: DMA_CFG_B: 0x001b
> > > [1.352991] tegra-nand 70008000.nand: FIFO_CTRL: 0xaa00
> > > [reset]
> > > [1.358559] tegra-nand 70008000.nand: Tegra NAND controller register
> > > dump
> > > [1.365352] tegra-nand 70008000.nand: COMMAND: 0x0084
> > > [1.370744] tegra-nand 70008000.nand: STATUS: 0x0101
> > > [1.376060] tegra-nand 70008000.nand: ISR: 0x0100
> > > [1.381105] tegra-nand 70008000.nand: IER: 0x
> > > [1.386161] tegra-nand 70008000.nand: CONFIG: 0x1003
> > > [1.391466] tegra-nand 70008000.nand: TIMING: 0x
> > > [1.396782] tegra-nand 70008000.nand: TIMING2: 0x
> > > [1.402174] tegra-nand 70008000.nand: CMD_REG1: 0x
> > > [1.407664] tegra-nand 70008000.nand: CMD_REG2: 0x
> > > [1.413156] tegra-nand 70008000.nand: ADDR_REG1: 0x
> > > [1.418722] tegra-nand 70008000.nand: ADDR_REG2: 0x
> > > [1.424297] tegra-nand 70008000.nand: DMA_MST_CTRL: 0x2400
> > > [1.430123] tegra-nand 70008000.nand: DMA_CFG_A: 0x
> > > [1.435698] tegra-nand 70008000.nand: DMA_CFG_B: 0x
> > > [1.441264] tegra-nand 70008000.nand: FIFO_CTRL: 0xaa00
> > 
> > Alright, then indeed it's not really worth to bother with HW resetting
> > here. Probably only a kernel module reload or a reboot will help if HW is
> > hung. Maybe NAND controller / chip recovering is something that NAND core
> > should be handling in a such case by providing a nand_controller_reset()
> > hook?
> I don't see what the core could do to help with that. We'd end up with
> a new hook implemented by the controller that would be called by the
> controller driver when it knows it's safe to reset the controller. So,
> why bother exposing that in the core?

Giving a driver more flexibility is always a good thing. I'm not really 
familiar with mtd/ and maybe indeed it doesn't make much sense to move HW 
resetting to NAND core, though it looked to me that it should be always safe 
for NAND core to initiate HW resetting after IO failure and hence would be 
cleaner and nicer to have a unified HW reset management rather than to have 
each driver to do its own thing.

Re: [PATCH v2 5/5] memory: tegra: Introduce Tegra20 EMC driver

[Dmitry Osipenko]

On Monday, 11 June 2018 14:35:03 MSK Thierry Reding wrote:
> On Wed, Jun 06, 2018 at 04:42:01PM +0300, Dmitry Osipenko wrote:
> > On 06.06.2018 14:02, Thierry Reding wrote:
> > > On Mon, Jun 04, 2018 at 01:36:54AM +0300, Dmitry Osipenko wrote:
> [...]
> 
> > >> +struct tegra_emc {
> > >> +struct device *dev;
> > >> +struct notifier_block clk_nb;
> > >> +struct clk *backup_clk;
> > >> +struct clk *emc_mux;
> > >> +struct clk *pll_m;
> > >> +struct clk *clk;
> > >> +void __iomem *regs;
> > >> +
> > >> +struct completion clk_handshake_complete;
> > >> +int irq;
> > >> +
> > >> +struct emc_timing *timings;
> > >> +unsigned int num_timings;
> > >> +};
> > >> +
> > >> +static irqreturn_t tegra_emc_isr(int irq, void *data)
> > >> +{
> > >> +struct tegra_emc *emc = data;
> > >> +u32 intmask = EMC_CLKCHANGE_COMPLETE_INT;
> > >> +u32 status;
> > >> +
> > >> +status = readl_relaxed(emc->regs + EMC_INTSTATUS) & intmask;
> > >> +if (!status)
> > >> +return IRQ_NONE;
> > >> +
> > >> +/* clear interrupts */
> > >> +writel_relaxed(status, emc->regs + EMC_INTSTATUS);
> > > 
> > > Do we really want to just clear the handshake complete interrupt or do
> > > we want to clear all of them? Perhaps we should also warn if there are
> > > other interrupts that we're not handling? Currently we'd only get some
> > > warning if another interrupt triggered without the handshake complete
> > > one triggering at the same time, but couldn't there be others asserted
> > > along with the handshake complete interrupt? In which case we'd just
> > > be ignoring them. Or perhaps not clearing it would get the ISR run
> > > immediately again and produce the "nobody cared" warning?
> > 
> > Yes, we really want to just clear the handshake-complete interrupt. No, we
> > shouldn't warn about other interrupts because IRQ subsys does it for us.
> > Other interrupts shouldn't be asserted because we disabled them with the
> > interrupts masking in emc_setup_hw(). Other interrupts can only be
> > asserted in a case of a bug, there will be a "nobody cared" warning and
> > interrupt will be disabled, this is exactly what we want in that case.
> 
> Okay, that's what I was suspecting might happen. Seems fine, then.
> 
> > >> +static int cmp_timings(const void *_a, const void *_b)
> > >> +{
> > >> +const struct emc_timing *a = _a;
> > >> +const struct emc_timing *b = _b;
> > >> +
> > >> +if (a->rate < b->rate)
> > >> +return -1;
> > >> +else if (a->rate == b->rate)
> > >> +return 0;
> > >> +else
> > >> +return 1;
> > > 
> > > Nit, I tend to
> > 
> > Tend to..?
> 
> Looks like I got distracted at this point. =) What I meant to say is
> that I tend to not use if ... else if ... else for things that do
> immediately return. The above is equivalent to the below, which I think
> is much easier to read:
> 
>   if (a->rate < b->rate)
>   return -1;
> 
>   if (a->rate > b->rate)
>   return 1;
> 
>   return 0;
> 

Okay.

> > >> +}
> > >> +
> > >> +static int tegra_emc_load_timings_from_dt(struct tegra_emc *emc,
> > >> +  struct device_node *node)
> > >> +{
> > >> +struct device_node *child;
> > >> +struct emc_timing *timing;
> > >> +int child_count;
> > >> +int err;
> > >> +
> > >> +child_count = of_get_child_count(node);
> > > 
> > > It's unfortunate that of_get_child_count() doesn't return unsigned int,
> > > there's no reason why this would have to be signed.
> > 
> > Patches are welcome ;)
> 
> Yeah, just a random drive-by comment. Please ignore. =)
> 
> > >> +if (!child_count) {
> > >> +dev_err(emc->dev, "no memory timings in DT node\n");
> > >> +return -ENOENT;
> > >> +

Re: [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On Monday, 11 June 2018 14:50:13 MSK Boris Brezillon wrote:
> On Mon, 11 Jun 2018 14:45:45 +0300
> 
> Dmitry Osipenko  wrote:
> > On Sunday, 10 June 2018 18:32:02 MSK Boris Brezillon wrote:
> > > On Sun, 10 Jun 2018 18:00:06 +0300
> > > 
> > > Dmitry Osipenko  wrote:
> > > > > >> That seems a lot of work for a code path I do not intend to ever
> > > > > >> use
> > > > > >> 
> > > > > >> :-)
> > > > > > 
> > > > > > Are you sure that resetting HW resets the timing and other
> > > > > > registers
> > > > > > configuration? Reset implementation is HW-specific, like for
> > > > > > example
> > > > > > in a
> > > > > > case of a video decoder the registers state is re-intialized on HW
> > > > > > reset,
> > > > > > but registers configuration is untouched in a case of resetting
> > > > > > GPU.
> > > > > > I'd
> > > > > > suggest to check whether NAND controller resetting affects the HW
> > > > > > configuration.
> > > > > 
> > > > > It seems all registers are set back to their documented reset value:
> > > > > 
> > > > > [boot loader/ROM initialized values]
> > > > > [1.270253] tegra-nand 70008000.nand: Tegra NAND controller
> > > > > register
> > > > > dump
> > > > > [1.277051] tegra-nand 70008000.nand: COMMAND: 0x66880104
> > > > > [1.282457] tegra-nand 70008000.nand: STATUS: 0x0101
> > > > > [1.287763] tegra-nand 70008000.nand: ISR: 0x01000120
> > > > > [1.292818] tegra-nand 70008000.nand: IER: 0x
> > > > > [1.297863] tegra-nand 70008000.nand: CONFIG: 0x0084
> > > > > [1.303181] tegra-nand 70008000.nand: TIMING: 0x0504
> > > > > [1.308486] tegra-nand 70008000.nand: TIMING2: 0x0003
> > > > > [1.313897] tegra-nand 70008000.nand: CMD_REG1: 0x
> > > > > [1.319377] tegra-nand 70008000.nand: CMD_REG2: 0x0030
> > > > > [1.324868] tegra-nand 70008000.nand: ADDR_REG1: 0x0300
> > > > > [1.330435] tegra-nand 70008000.nand: ADDR_REG2: 0x
> > > > > [1.336011] tegra-nand 70008000.nand: DMA_MST_CTRL: 0x0414
> > > > > [1.341838] tegra-nand 70008000.nand: DMA_CFG_A: 0x0fff
> > > > > [1.347415] tegra-nand 70008000.nand: DMA_CFG_B: 0x001b
> > > > > [1.352991] tegra-nand 70008000.nand: FIFO_CTRL: 0xaa00
> > > > > [reset]
> > > > > [1.358559] tegra-nand 70008000.nand: Tegra NAND controller
> > > > > register
> > > > > dump
> > > > > [1.365352] tegra-nand 70008000.nand: COMMAND: 0x0084
> > > > > [1.370744] tegra-nand 70008000.nand: STATUS: 0x0101
> > > > > [1.376060] tegra-nand 70008000.nand: ISR: 0x0100
> > > > > [1.381105] tegra-nand 70008000.nand: IER: 0x
> > > > > [1.386161] tegra-nand 70008000.nand: CONFIG: 0x1003
> > > > > [1.391466] tegra-nand 70008000.nand: TIMING: 0x
> > > > > [1.396782] tegra-nand 70008000.nand: TIMING2: 0x
> > > > > [1.402174] tegra-nand 70008000.nand: CMD_REG1: 0x
> > > > > [1.407664] tegra-nand 70008000.nand: CMD_REG2: 0x
> > > > > [1.413156] tegra-nand 70008000.nand: ADDR_REG1: 0x
> > > > > [1.418722] tegra-nand 70008000.nand: ADDR_REG2: 0x
> > > > > [1.424297] tegra-nand 70008000.nand: DMA_MST_CTRL: 0x2400
> > > > > [1.430123] tegra-nand 70008000.nand: DMA_CFG_A: 0x
> > > > > [1.435698] tegra-nand 70008000.nand: DMA_CFG_B: 0x
> > > > > [1.441264] tegra-nand 70008000.nand: FIFO_CTRL: 0xaa00
> > > > 
> > > > Alright, then indeed it's not really worth to bother with HW resetting
> > > > here. Probably only a kernel module reload or a reboot will help if HW
> > > > is
> > > > hung. Maybe NAND controller / chip recovering is something that NAND
> > > > core
> > > > should be handling in a such case by providing a
> > > > nand_controller_reset()
> > > > hook?
> > > 
> > > I don't see what the core could do to help with that. We'd end up with
> > > a new hook implemented by the controller that would be called by the
> > > controller driver when it knows it's safe to reset the controller. So,
> > > why bother exposing that in the core?
> > 
> > Giving a driver more flexibility is always a good thing. I'm not really
> > familiar with mtd/ and maybe indeed it doesn't make much sense to move HW
> > resetting to NAND core, though it looked to me that it should be always
> > safe for NAND core to initiate HW resetting after IO failure and hence
> > would be cleaner and nicer to have a unified HW reset management rather
> > than to have each driver to do its own thing.
> 
> No really, the NAND core can't know when it's appropriate to reset the
> controller, and what this reset will do, hence it doesn't know if the
> chips connected to the controller should also be reset.

Okay!

Re: [PATCH v2 5/5] memory: tegra: Introduce Tegra20 EMC driver

[Dmitry Osipenko]

On Monday, 11 June 2018 14:41:33 MSK Thierry Reding wrote:
> On Mon, Jun 11, 2018 at 01:35:03PM +0200, Thierry Reding wrote:
> > On Wed, Jun 06, 2018 at 04:42:01PM +0300, Dmitry Osipenko wrote:
> > > On 06.06.2018 14:02, Thierry Reding wrote:
> > > > On Mon, Jun 04, 2018 at 01:36:54AM +0300, Dmitry Osipenko wrote:
> [...]
> 
> > > >> +  if (!child_count) {
> > > >> +  dev_err(emc->dev, "no memory timings in DT node\n");
> > > >> +  return -ENOENT;
> > > >> +  }
> > > >> +
> > > >> +  emc->timings = devm_kcalloc(emc->dev, child_count, 
sizeof(*timing),
> > > >> +  GFP_KERNEL);
> > > >> +  if (!emc->timings)
> > > >> +  return -ENOMEM;
> > > >> +
> > > >> +  emc->num_timings = child_count;
> > > >> +  timing = emc->timings;
> > > >> +
> > > >> +  for_each_child_of_node(node, child) {
> > > >> +  err = load_one_timing_from_dt(emc, timing++, child);
> > > >> +  if (err) {
> > > >> +  of_node_put(child);
> > > >> +  return err;
> > > >> +  }
> > > >> +  }
> > > >> +
> > > >> +  sort(emc->timings, emc->num_timings, sizeof(*timing), 
cmp_timings,
> > > >> +   NULL);
> > > >> +
> > > >> +  return 0;
> > > >> +}
> > > >> +
> > > >> +static struct device_node *
> > > >> +tegra_emc_find_node_by_ram_code(struct tegra_emc *emc, u32 ram_code)
> > > >> +{
> > > >> +  struct device_node *np;
> > > >> +  int err;
> > > >> +
> > > >> +  for_each_child_of_node(emc->dev->of_node, np) {
> > > >> +  u32 value;
> > > >> +
> > > >> +  err = of_property_read_u32(np, "nvidia,ram-code", 
> > > >> );
> > > >> +  if (err || value != ram_code)
> > > >> +  continue;
> > > >> +
> > > >> +  return np;
> > > >> +  }
> > > >> +
> > > >> +  dev_info(emc->dev, "no memory timings for RAM code %u found in
> > > >> DT\n",
> > > >> +   ram_code);
> > > > 
> > > > This seems like it should be dev_warn() or perhaps even dev_err()
> > > > given
> > > > that the result of it is the driver failing to probe. dev_info() may
> > > > go
> > > > unnoticed.
> > > 
> > > Absence of memory timings is a valid case, hence dev_info() suit well
> > > here.
> > > 
> > > I can't see anything wrong with returning a errno if driver has nothing
> > > to do and prefer to keep it because in that case managed resources
> > > would be free'd by the driver core, though returning '0' also would
> > > work.
> > 
> > I disagree. A driver failing to probe will show up as a kernel log entry
> > and is something that people will have to whitelist if they're filtering
> > for error messages in the boot log.
> > 
> > I think it's more user-friendly to just let the driver succeed the probe
> > in an expected case, even if that means there's really nothing to do. If
> > you're really concerned about the managed resources staying around, I
> > think you could probably get rid of them explicitly. By the looks of it
> > devres_release_all() isn't an exported symbol, so it can't be called
> > from driver code, but perhaps that's something that we can change.
> 
> Maybe an easier way to avoid keeping the managed resources around would
> be to move the check a little further up. That way, we can abort earlier
> if no EMC timings are available, before any resources are even
> allocated.
> 
> The tegra_emc_find_node_by_ram_code() function would need to take a
> struct device * instead of struct tegra_emc *, but otherwise it should
> work fine.

Good point, thank you!

Re: [PATCH v2 5/5] memory: tegra: Introduce Tegra20 EMC driver

[Dmitry Osipenko]

On Monday, 11 June 2018 18:53:43 MSK Thierry Reding wrote:
> On Mon, Jun 11, 2018 at 04:06:41PM +0300, Dmitry Osipenko wrote:
> > On Monday, 11 June 2018 14:35:03 MSK Thierry Reding wrote:
> > > On Wed, Jun 06, 2018 at 04:42:01PM +0300, Dmitry Osipenko wrote:
> > > > On 06.06.2018 14:02, Thierry Reding wrote:
> > > > > On Mon, Jun 04, 2018 at 01:36:54AM +0300, Dmitry Osipenko wrote:
> [...]
> 
> > > > >> +} else {
> > > > >> +init_completion(>clk_handshake_complete);
> > > > >> +
> > > > >> +err = devm_request_irq(>dev, emc->irq, 
> > > > >> tegra_emc_isr,
> > 
> > 0,
> > 
> > > > >> +   dev_name(>dev), emc);
> > > > >> +if (err < 0) {
> > > > >> +dev_err(>dev, "failed to request IRQ#%u: 
> > > > >> %d\n",
> > > > >> +emc->irq, err);
> > > > >> +return err;
> > > > >> +}
> > > > >> +}
> > > > >> +
> > > > >> +emc->pll_m = clk_get_sys(NULL, "pll_m");
> > > > >> +if (IS_ERR(emc->pll_m)) {
> > > > >> +err = PTR_ERR(emc->pll_m);
> > > > >> +dev_err(>dev, "failed to get pll_m: %d\n", err);
> > > > >> +return err;
> > > > >> +}
> > > > >> +
> > > > >> +emc->backup_clk = clk_get_sys(NULL, "pll_p");
> > > > >> +if (IS_ERR(emc->backup_clk)) {
> > > > >> +err = PTR_ERR(emc->backup_clk);
> > > > >> +dev_err(>dev, "failed to get pll_p: %d\n", err);
> > > > >> +goto put_pll_m;
> > > > >> +}
> > > > >> +
> > > > >> +emc->clk = clk_get_sys(NULL, "emc");
> > > > >> +if (IS_ERR(emc->clk)) {
> > > > >> +err = PTR_ERR(emc->clk);
> > > > >> +dev_err(>dev, "failed to get emc: %d\n", err);
> > > > >> +goto put_backup;
> > > > >> +}
> > > > > 
> > > > > Instead of using clk_get_sys(), why not specify these in the DT with
> > > > > proper names for context ("emc", "pll", "backup")? Again, I don't
> > > > > think
> > > > > we have to worry about backwards-compatibility here since there can
> > > > > be
> > > > > no regression.
> > > > 
> > > > I don't think that "pll" and "backup" could be placed in DT because it
> > > > is
> > > > a pure software-driver descriptio in
> > > 
> > > DT. There are other cases, like for display, where we list clocks in the
> > > DT that aren't strictly inputs to a hardware block. But they are related
> > > to the functionality of the block, so I think it makes sense to list
> > > them as well.
> > > 
> > > In this particular case, the PLL is what drives the memory banks, which
> > > is the think that the EMC controls, right? So that itself is reason
> > > enough, in my opinion, to list it in DT. Much the same goes for the
> > > backup clock, which is really just the PLL for some transient state
> > > where the normal PLL is being reconfigured.
> > 
> > PLL itself shouldn't really matter. EMC doesn't control PLL, but only
> > interacts with the Clock-and-Reset controller. This means that we could
> > use
> > PLL_C instead of PLL_M if we wanted. Though PLL_M is meant to be a "memory
> > PLL".
> > 
> > To me a selection of a parent clock is a pure software description that is
> > wrong to be placed in DT.
> 
> I think of it as configuration rather than software description. The
> problem that we're trying to solve with DT is primarily one of
> configuration and parameterization. The idea is that we describe the
> interfaces of some hardware module and then specify which resources
> to use as inputs and/or outputs for those interfaces.
> 
> So in this case we can define that in order to perform its function the
> EMC driver needs to somehow control a PLL that drives the memory. Even
> if the EMC hardware itself doesn't contro

Re: [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Dmitry Osipenko]

On Saturday, 9 June 2018 00:51:01 MSK Stefan Agner wrote:
> On 01.06.2018 11:20, Dmitry Osipenko wrote:
> > On 01.06.2018 01:16, Stefan Agner wrote:
> >> Add support for the NAND flash controller found on NVIDIA
> >> Tegra 2 SoCs. This implementation does not make use of the
> >> command queue feature. Regular operations/data transfers are
> >> done in PIO mode. Page read/writes with hardware ECC make
> >> use of the DMA for data transfer.
> >> 
> >> Signed-off-by: Lucas Stach 
> >> Signed-off-by: Stefan Agner 
> >> ---
> >> 
> >>  MAINTAINERS   |7 +
> >>  drivers/mtd/nand/raw/Kconfig  |6 +
> >>  drivers/mtd/nand/raw/Makefile |1 +
> >>  drivers/mtd/nand/raw/tegra_nand.c | 1143 +
> >>  4 files changed, 1157 insertions(+)
> >>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
> >> 
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 58b9861ccf99..c2e5571c85d4 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -13844,6 +13844,13 @@ M:Laxman Dewangan 
> >> 
> >>  S:Supported
> >>  F:drivers/input/keyboard/tegra-kbc.c
> >> 
> >> +TEGRA NAND DRIVER
> >> +M:Stefan Agner 
> >> +M:Lucas Stach 
> >> +S:Maintained
> >> +F:Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> +F:drivers/mtd/nand/raw/tegra_nand.c
> >> +
> >> 
> >>  TEGRA PWM DRIVER
> >>  M:Thierry Reding 
> >>  S:Supported
> >> 
> >> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> >> index 19a2b283fbbe..e9093f52371e 100644
> >> --- a/drivers/mtd/nand/raw/Kconfig
> >> +++ b/drivers/mtd/nand/raw/Kconfig
> >> @@ -534,4 +534,10 @@ config MTD_NAND_MTK
> >> 
> >>  Enables support for NAND controller on MTK SoCs.
> >>  This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> >> 
> >> +config MTD_NAND_TEGRA
> >> +  tristate "Support for NAND controller on NVIDIA Tegra"
> >> +  depends on ARCH_TEGRA || COMPILE_TEST
> >> +  help
> >> +Enables support for NAND flash controller on NVIDIA Tegra SoC.
> >> +
> >> 
> >>  endif # MTD_NAND
> >> 
> >> diff --git a/drivers/mtd/nand/raw/Makefile
> >> b/drivers/mtd/nand/raw/Makefile
> >> index 165b7ef9e9a1..d5a5f9832b88 100644
> >> --- a/drivers/mtd/nand/raw/Makefile
> >> +++ b/drivers/mtd/nand/raw/Makefile
> >> @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)   +=
> >> hisi504_nand.o
> >> 
> >>  obj-$(CONFIG_MTD_NAND_BRCMNAND)   += brcmnand/
> >>  obj-$(CONFIG_MTD_NAND_QCOM)   += qcom_nandc.o
> >>  obj-$(CONFIG_MTD_NAND_MTK)+= mtk_ecc.o mtk_nand.o
> >> 
> >> +obj-$(CONFIG_MTD_NAND_TEGRA)  += tegra_nand.o
> >> 
> >>  nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
> >>  nand-objs += nand_amd.o
> >> 
> >> diff --git a/drivers/mtd/nand/raw/tegra_nand.c
> >> b/drivers/mtd/nand/raw/tegra_nand.c new file mode 100644
> >> index ..e9664f2938a3
> >> --- /dev/null
> >> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> >> @@ -0,0 +1,1143 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2018 Stefan Agner 
> >> + * Copyright (C) 2014-2015 Lucas Stach 
> >> + * Copyright (C) 2012 Avionic Design GmbH
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#define CMD   0x00
> >> +#define   CMD_GO  BIT(31)
> >> +#define   CMD_CLE BIT(30)
> >> +#define   CMD_ALE BIT(29)
> >> +#define   CMD_PIO BIT(28)
> >> +#define   CMD_TX  BIT(27)
> >> +#define   CMD_RX  BIT(26)
> >> +#define   CMD_SEC_CMD BIT(25)
> >> +#defin

Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties

[Dmitry Osipenko]

On 28.05.2018 16:15, Ville Syrjälä wrote:
> On Sat, May 26, 2018 at 06:56:22PM +0300, Dmitry Osipenko wrote:
>> Color keying is the action of replacing pixels matching a given color
>> (or range of colors) with transparent pixels in an overlay when
>> performing blitting. Depending on the hardware capabilities, the
>> matching pixel can either become fully transparent or gain adjustment
>> of the pixels component values.
>>
>> Color keying is found in a large number of devices whose capabilities
>> often differ, but they still have enough common features in range to
>> standardize color key properties. This commit adds nine generic DRM plane
>> properties related to the color keying to cover various HW capabilities.
>>
>> This patch is based on the initial work done by Laurent Pinchart, most of
>> credits for this patch goes to him.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/gpu/drm/drm_atomic.c |  36 ++
>>  drivers/gpu/drm/drm_blend.c  | 229 +++
>>  include/drm/drm_blend.h  |   3 +
>>  include/drm/drm_plane.h  |  77 
>>  4 files changed, 345 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 895741e9cd7d..5b808cb68654 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -799,6 +799,24 @@ static int drm_atomic_plane_set_property(struct 
>> drm_plane *plane,
>>  state->rotation = val;
>>  } else if (property == plane->zpos_property) {
>>  state->zpos = val;
>> +} else if (property == plane->colorkey.mode_property) {
>> +state->colorkey.mode = val;
>> +} else if (property == plane->colorkey.min_property) {
>> +state->colorkey.min = val;
>> +} else if (property == plane->colorkey.max_property) {
>> +state->colorkey.max = val;
>> +} else if (property == plane->colorkey.format_property) {
>> +state->colorkey.format = val;
>> +} else if (property == plane->colorkey.mask_property) {
>> +state->colorkey.mask = val;
>> +} else if (property == plane->colorkey.inverted_match_property) {
>> +state->colorkey.inverted_match = val;
>> +} else if (property == plane->colorkey.replacement_mask_property) {
>> +state->colorkey.replacement_mask = val;
>> +} else if (property == plane->colorkey.replacement_value_property) {
>> +state->colorkey.replacement_value = val;
>> +} else if (property == plane->colorkey.replacement_format_property) {
>> +state->colorkey.replacement_format = val;
>>  } else if (property == plane->color_encoding_property) {
>>  state->color_encoding = val;
>>  } else if (property == plane->color_range_property) {
>> @@ -864,6 +882,24 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>>  *val = state->rotation;
>>  } else if (property == plane->zpos_property) {
>>  *val = state->zpos;
>> +} else if (property == plane->colorkey.mode_property) {
>> +*val = state->colorkey.mode;
>> +} else if (property == plane->colorkey.min_property) {
>> +*val = state->colorkey.min;
>> +} else if (property == plane->colorkey.max_property) {
>> +*val = state->colorkey.max;
>> +} else if (property == plane->colorkey.format_property) {
>> +*val = state->colorkey.format;
>> +} else if (property == plane->colorkey.mask_property) {
>> +*val = state->colorkey.mask;
>> +} else if (property == plane->colorkey.inverted_match_property) {
>> +*val = state->colorkey.inverted_match;
>> +} else if (property == plane->colorkey.replacement_mask_property) {
>> +*val = state->colorkey.replacement_mask;
>> +} else if (property == plane->colorkey.replacement_value_property) {
>> +*val = state->colorkey.replacement_value;
>> +} else if (property == plane->colorkey.replacement_format_property) {
>> +*val = state->colorkey.replacement_format;
>>  } else if (property == plane->color_encoding_property) {
>>  *val = state->color_encoding;
>>  } else if (property == plane->color_range_property) {
>> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
>> index a16a74d7e15e..05e

Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties

[Dmitry Osipenko]

On 29.05.2018 02:48, Dmitry Osipenko wrote:
> inversion=true" if mask has form of 0x11000111, though this could be not

For clarity: I meant s/0x11000111/0xFF000FFF/.

Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties

[Dmitry Osipenko]

On 26.05.2018 19:18, Laurent Pinchart wrote:
> On Saturday, 26 May 2018 19:16:54 EEST Laurent Pinchart wrote:
>> Hi Dimitri,
> 
> And sorry for the spelling mistake :-/

That's also a kinda correct spelling. No worries ;)

[PATCH v1] media: dt: bindings: tegra-vde: Document new optional Memory Client reset property

[Dmitry Osipenko]

Recently binding of the Memory Controller has been extended, exposing
the Memory Client reset controls and hence it is now a reset controller.
Tegra video-decoder device is among the Memory Controller reset users,
document the new optional VDE HW reset property.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 .../devicetree/bindings/media/nvidia,tegra-vde.txt| 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt 
b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
index 470237ed6fe5..7302e949e662 100644
--- a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
+++ b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
@@ -27,9 +27,15 @@ Required properties:
   - sxe
 - clocks : Must include the following entries:
   - vde
-- resets : Must include the following entries:
+- resets : Must contain an entry for each entry in reset-names.
+- reset-names : Should include the following entries:
   - vde
 
+Optional properties:
+- resets : Must contain an entry for each entry in reset-names.
+- reset-names : Must include the following entries:
+  - mc
+
 Example:
 
 video-codec@6001a000 {
@@ -51,5 +57,6 @@ video-codec@6001a000 {
 ; /* SXE interrupt */
interrupt-names = "sync-token", "bsev", "sxe";
clocks = <_car TEGRA20_CLK_VDE>;
-   resets = <_car 61>;
+   reset-names = "vde", "mc";
+   resets = <_car 61>, < TEGRA20_MC_RESET_VDE>;
 };
-- 
2.17.0

[PATCH v1] memory: tegra: Correct driver probe order

[Dmitry Osipenko]

The Reset Controller should be registered in the end of probe, otherwise
Memory Controller device goes away if IRQ requesting fails and the Reset
Controller stays registered. To avoid having to unwind the MC probing in
a case of SMMU probe failure, let's simply print the error message without
failing the MC probe. This allows us to just move the Reset Controller
registering before the SMMU registration, reducing code churning. Also
let's not fail MC probe in a case of Reset Controller registration failure
as it doesn't prevent the MC driver to work.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
 drivers/memory/tegra/mc.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 22ec959db7d3..8ef95e34b5c7 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -692,13 +692,6 @@ static int tegra_mc_probe(struct platform_device *pdev)
return err;
}
 
-   err = tegra_mc_reset_setup(mc);
-   if (err < 0) {
-   dev_err(>dev, "failed to register reset controller: %d\n",
-   err);
-   return err;
-   }
-
mc->irq = platform_get_irq(pdev, 0);
if (mc->irq < 0) {
dev_err(>dev, "interrupt not specified\n");
@@ -717,13 +710,16 @@ static int tegra_mc_probe(struct platform_device *pdev)
return err;
}
 
+   err = tegra_mc_reset_setup(mc);
+   if (err < 0)
+   dev_err(>dev, "failed to register reset controller: %d\n",
+   err);
+
if (IS_ENABLED(CONFIG_TEGRA_IOMMU_SMMU)) {
mc->smmu = tegra_smmu_probe(>dev, mc->soc->smmu, mc);
-   if (IS_ERR(mc->smmu)) {
+   if (IS_ERR(mc->smmu))
dev_err(>dev, "failed to probe SMMU: %ld\n",
PTR_ERR(mc->smmu));
-   return PTR_ERR(mc->smmu);
-   }
}
 
return 0;
-- 
2.17.0

Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties

[Dmitry Osipenko]

On 26.05.2018 19:16, Laurent Pinchart wrote:
> Hi Dimitri,
> 
> Thank you for the patch.
> 
> I'll review this in details, but as this patch is based on the "[PATCH/RFC 
> 1/4] drm: Add colorkey properties" patch I've submitted, please retain the 
> authorship, both in the Signed-off-by line, and in the patch author in git.
Okay. /I think/ I've seen requests to do the other way around for the picked up
and re-worked patches, though I don't mind at all to keep your authorship. I'll
change the authorship in the next iteration. Waiting for you review comments,
thanks.

[RFC PATCH v2 0/2] Implement standard color keying properties for DRM planes

[Dmitry Osipenko]

Hello, DRM maintainers!

Laurent Pinchart kindly agreed to allow me to pick up his work on
the generic colorkey DRM plane property [0]. I've reworked the original
patch a tad, hopefully making it flexible enough to cover various HW
capabilities.

Changes I've made:

- Some code clean up and reshuffle.

- Took into account some the Ville's Syrjälä review comments to [0].

- The number of common DRM colorkey properties grows from 4 to 9.
  New properties:
- colorkey.mask
- colorkey.format
- colorkey.inverted-match
- colorkey.replacement-mask
- colorkey.replacement-format

  Renamed properties:
- colorkey.value -> colorkey.replacement-value

- colorkey.mode userspace-property ENUM's got a bit more explicit
  names, like "src" -> "src-match-src-replace".

- No driver-specific modes / properties allowed, all unsupported
  features are simply rejected by the drivers.

This patchset includes initial colorkey property implementation for the
older NVIDIA Tegra's.

Please review, thanks.

[0] https://lists.freedesktop.org/archives/dri-devel/2017-December/160510.html

Dmitry Osipenko (2):
  drm: Add generic colorkey properties
  drm/tegra: plane: Implement generic colorkey property for older
Tegra's

 drivers/gpu/drm/drm_atomic.c  |  36 ++
 drivers/gpu/drm/drm_blend.c   | 229 ++
 drivers/gpu/drm/tegra/dc.c|  31 +
 drivers/gpu/drm/tegra/dc.h|   7 ++
 drivers/gpu/drm/tegra/plane.c | 147 ++
 drivers/gpu/drm/tegra/plane.h |   1 +
 include/drm/drm_blend.h   |   3 +
 include/drm/drm_plane.h   |  77 
 8 files changed, 531 insertions(+)

-- 
2.17.0