* [PATCH 2/2] drm/aspeed: Add DVO output option to GFX driver
From: Timothy Pearson @ 2019-05-02 21:51 UTC (permalink / raw)
To: linux-aspeed
The AST2500 offers an alternate GFX output mode over DVO.
Enable DVO or VGA output mode conditionally based on two new
device tree properties, output-vga and output-dvo.
Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
---
drivers/gpu/drm/aspeed/aspeed_gfx.h | 6 ++++++
drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 29 +++++++++++++++++++++++------
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 17 ++++++++++++++++-
3 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
index b34c97613aaf..6f9bc01191c0 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
@@ -14,6 +14,8 @@ struct aspeed_gfx {
struct drm_simple_display_pipe pipe;
struct drm_connector connector;
struct drm_fbdev_cma *fbdev;
+
+ u8 output_mode;
};
int aspeed_gfx_create_pipe(struct drm_device *drm);
@@ -105,3 +107,7 @@ int aspeed_gfx_create_output(struct drm_device *drm);
/* Default Threshold Seting */
#define G5_CRT_THROD_VAL (CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3C))
+
+/* Output mode */
+#define OUTPUT_VGA 0x1
+#define OUTPUT_DVO 0x2
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
index 15db9e426ec4..ee16f9011d70 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
// Copyright 2018 IBM Corporation
+// Copyright 2019 Raptor Engineering, LLC
#include <linux/clk.h>
#include <linux/reset.h>
@@ -59,11 +60,21 @@ static void aspeed_gfx_enable_controller(struct aspeed_gfx *priv)
u32 ctrl1 = readl(priv->base + CRT_CTRL1);
u32 ctrl2 = readl(priv->base + CRT_CTRL2);
- /* SCU2C: set DAC source for display output to Graphics CRT (GFX) */
- regmap_update_bits(priv->scu, 0x2c, BIT(16), BIT(16));
+ if (priv->output_mode & OUTPUT_VGA) {
+ /* SCU2C: set DAC source for display output to Graphics CRT (GFX) */
+ regmap_update_bits(priv->scu, 0x2c, BIT(16), BIT(16));
+ }
+ if (priv->output_mode & OUTPUT_DVO) {
+ /* SCU2C: set DVO source for display output to Graphics CRT (GFX) */
+ regmap_update_bits(priv->scu, 0x2c, BIT(18), BIT(18));
+ }
writel(ctrl1 | CRT_CTRL_EN, priv->base + CRT_CTRL1);
- writel(ctrl2 | CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
+
+ if (priv->output_mode & OUTPUT_VGA)
+ writel(ctrl2 | CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
+ if (priv->output_mode & OUTPUT_DVO)
+ writel(ctrl2 | CRT_CTRL_DVO_EN, priv->base + CRT_CTRL2);
}
static void aspeed_gfx_disable_controller(struct aspeed_gfx *priv)
@@ -72,9 +83,15 @@ static void aspeed_gfx_disable_controller(struct aspeed_gfx *priv)
u32 ctrl2 = readl(priv->base + CRT_CTRL2);
writel(ctrl1 & ~CRT_CTRL_EN, priv->base + CRT_CTRL1);
- writel(ctrl2 & ~CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
-
- regmap_update_bits(priv->scu, 0x2c, BIT(16), 0);
+ if (priv->output_mode & OUTPUT_VGA)
+ writel(ctrl2 & ~CRT_CTRL_DAC_EN, priv->base + CRT_CTRL2);
+ if (priv->output_mode & OUTPUT_DVO)
+ writel(ctrl2 & ~CRT_CTRL_DVO_EN, priv->base + CRT_CTRL2);
+
+ if (priv->output_mode & OUTPUT_VGA)
+ regmap_update_bits(priv->scu, 0x2c, BIT(16), 0);
+ if (priv->output_mode & OUTPUT_DVO)
+ regmap_update_bits(priv->scu, 0x2c, BIT(18), 0);
}
static void aspeed_gfx_crtc_mode_set_nofb(struct aspeed_gfx *priv)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index 7e9072fd0ef0..17a22dd0922a 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
// Copyright 2018 IBM Corporation
+// Copyright 2019 Raptor Engineering, LLC
#include <linux/clk.h>
#include <linux/dma-mapping.h>
@@ -50,7 +51,8 @@
* is the ARM's internal display controller.
*
* The driver only supports a simple configuration consisting of a 40MHz
- * pixel clock, fixed by hardware limitations, and the VGA output path.
+ * pixel clock, fixed by hardware limitations. It supports DVO output
+ * mode as well based on device tree configuration.
*
* The driver was written with the 'AST2500 Software Programming Guide' v17,
* which is available under NDA from ASPEED.
@@ -95,6 +97,7 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data)
static int aspeed_gfx_load(struct drm_device *drm)
{
struct platform_device *pdev = to_platform_device(drm->dev);
+ struct device_node *nc = drm->dev->of_node;
struct aspeed_gfx *priv;
struct resource *res;
int ret;
@@ -145,6 +148,18 @@ static int aspeed_gfx_load(struct drm_device *drm)
}
clk_prepare_enable(priv->clk);
+ if (of_property_read_bool(nc, "output-vga"))
+ priv->output_mode |= OUTPUT_VGA;
+ else if (of_property_read_bool(nc, "output-dvo"))
+ priv->output_mode |= OUTPUT_DVO;
+ else
+ priv->output_mode = OUTPUT_VGA;
+
+ if (priv->output_mode & OUTPUT_VGA)
+ DRM_INFO("Enabling VGA output\n");
+ if (priv->output_mode & OUTPUT_DVO)
+ DRM_INFO("Enabling DVO output\n");
+
/* Sanitize control registers */
writel(0, priv->base + CRT_CTRL1);
/* Preserve CRT_CTRL2[7:6] (DVO configuration) */
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] [v2] drm/aspeed: Preserve DVO configuration bits during initialization
From: Timothy Pearson @ 2019-05-02 21:51 UTC (permalink / raw)
To: linux-aspeed
GFX064 contains DVO enable and mode bits. These are hardware specific, configured
via the pinmux from the DT, and should not be cleared during startup.
Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
---
drivers/gpu/drm/aspeed/aspeed_gfx.h | 3 +++
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 5 ++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
index b7a986e49177..b34c97613aaf 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
// Copyright 2018 IBM Corporation
+// Copyright 2019 Raptor Engineering, LLC
#include <drm/drm_device.h>
#include <drm/drm_simple_kms_helper.h>
@@ -73,6 +74,8 @@ int aspeed_gfx_create_output(struct drm_device *drm);
/* CTRL2 */
#define CRT_CTRL_DAC_EN BIT(0)
+#define CRT_CTRL_DVO_MODE BIT(6)
+#define CRT_CTRL_DVO_EN BIT(7)
#define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK)
#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index 713a3975852b..7e9072fd0ef0 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -98,6 +98,7 @@ static int aspeed_gfx_load(struct drm_device *drm)
struct aspeed_gfx *priv;
struct resource *res;
int ret;
+ u32 reg;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -146,7 +147,9 @@ static int aspeed_gfx_load(struct drm_device *drm)
/* Sanitize control registers */
writel(0, priv->base + CRT_CTRL1);
- writel(0, priv->base + CRT_CTRL2);
+ /* Preserve CRT_CTRL2[7:6] (DVO configuration) */
+ reg = readl(priv->base + CRT_CTRL2) & (CRT_CTRL_DVO_MODE | CRT_CTRL_DVO_EN);
+ writel(reg, priv->base + CRT_CTRL2);
aspeed_gfx_setup_mode_config(drm);
--
2.11.0
^ permalink raw reply related
* [PATCH 7/7] media: aspeed: refine interrupt handling logic
From: Eddie James @ 2019-05-02 21:04 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-8-jae.hyun.yoo@linux.intel.com>
On 5/2/19 2:13 PM, Jae Hyun Yoo wrote:
> There are cases that interrupt bits are cleared by a 500ms delayed
> work which causes unnecessary irq calls. Also, the current
> interrupt handler returns IRQ_HANDLED always but it should return
> IRQ_NONE if there is any unhandled interrupt. So this commit
> refines the interrupt handling logic to fix these issues.
Thanks Jae, looks good.
Reviewed-by: Eddie James <eajames@linux.ibm.com>
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/media/platform/aspeed-video.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 8d0bb395e46d..98944a911998 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -488,6 +488,7 @@ static void aspeed_video_off(struct aspeed_video *video)
>
> /* Disable interrupts */
> aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
> + aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
>
> /* Turn off the relevant clocks */
> clk_disable(video->vclk);
> @@ -556,7 +557,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
> VE_INTERRUPT_MODE_DETECT, 0);
> aspeed_video_write(video, VE_INTERRUPT_STATUS,
> VE_INTERRUPT_MODE_DETECT);
> -
> + sts &= ~VE_INTERRUPT_MODE_DETECT;
> set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
> wake_up_interruptible_all(&video->wait);
> } else {
> @@ -601,12 +602,12 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
> VE_INTERRUPT_COMP_COMPLETE, 0);
> aspeed_video_write(video, VE_INTERRUPT_STATUS,
> VE_INTERRUPT_COMP_COMPLETE);
> -
> + sts &= ~VE_INTERRUPT_COMP_COMPLETE;
> if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
> aspeed_video_start_frame(video);
> }
>
> - return IRQ_HANDLED;
> + return sts ? IRQ_NONE : IRQ_HANDLED;
> }
>
> static void aspeed_video_check_and_set_polarity(struct aspeed_video *video)
^ permalink raw reply
* [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
From: Timothy Pearson @ 2019-05-02 19:37 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <e2388fbc-e664-4338-a0f5-d34a3621c3fb@www.fastmail.com>
----- Original Message -----
> From: "Andrew Jeffery" <andrew@aj.id.au>
> To: "Timothy Pearson" <tpearson@raptorengineering.com>
> Cc: "linux-aspeed" <linux-aspeed@lists.ozlabs.org>, "Ryan Chen" <ryan_chen@aspeedtech.com>
> Sent: Thursday, May 2, 2019 1:40:39 AM
> Subject: Re: [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
> On Thu, 2 May 2019, at 16:03, Timothy Pearson wrote:
>>
>>
>> ----- Original Message -----
>> > From: "Andrew Jeffery" <andrew@aj.id.au>
>> > To: "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed"
>> > <linux-aspeed@lists.ozlabs.org>, "Ryan Chen"
>> > <ryan_chen@aspeedtech.com>
>> > Sent: Thursday, May 2, 2019 12:51:00 AM
>> > Subject: Re: [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs
>> > on AST2500 devices
>>
>> > On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
>> >> There appears to be a significant error in the pinmux table starting on
>> >> page 127 of the AST2500 datasheet v1.6. Specifically, the COND2 (DVO)
>> >> requirement is incorrectly applied to multiple digital video input (DVI)
>> >> muxed pins, and no DVI-specific condition is provided. This results in
>> >> the serial devices incorrectly overriding the DVO pinmuxes and disabling
>> >> the DVO pins.
>> >>
>> >> Create a new condition code (COND6) for DVI enable, and update the most
>> >> seriously affected pins to use the new condition code.
>> >>
>> >> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
>> >> ---
>> >> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 17 +++++++++--------
>> >> 1 file changed, 9 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> >> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> >> index 6f357a11e89a..676f90d3c5f3 100644
>> >> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> >> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> >> @@ -29,6 +29,7 @@
>> >>
>> >> #define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
>> >> #define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
>> >> +#define COND6 { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 0, 0 }
>> >>
>> >> /* LHCR0 is offset from the end of the H8S/2168-compatible registers */
>> >> #define LHCR0 0x20
>> >> @@ -660,8 +661,8 @@ SSSF_PIN_DECL(T2, GPIOL0, NCTS1, SIG_DESC_SET(SCU84, 16));
>> >>
>> >> #define T1 89
>> >> #define T1_DESC SIG_DESC_SET(SCU84, 17)
>> >> -SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND2);
>> >> -SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND2);
>> >> +SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND6);
>> >> +SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND6);
>> >
>> > I feel like you didn't test this patch, because VPI_24_RSVD_DESC (the DVI
>> > condition)
>> > requires SCU90[5]=0b1, but your introduction of COND6 requires SCU90[5:4]=0b00
>> > for
>> > the mux configuration to succeed. This can't work - bit 5 of SCU90 can not
>> > simultaneously take the values 1 and 0.
>>
>> That's correct -- I don't have hardware that supports DVI available to
>> test with.
>
> Okay. In that case I'm not prepared to ACK changes here, much less changes with
> that fail in this way. The current implementation at least follows what is
> dictated by
> the programming guide and is at least correct in theory.
>
> As Ryan is not confident there are no negative side-effects to not following the
> configuration dictated by the programming guide, changes like this have a real
> up-hill battle on their hands.
>
> Cheers,
>
> Andrew
There is a negative effect right now in that enabling either UART will force disable the DVO pinmuxes. While I agree the patch needs additional work, as it stands right now DVO will not function simultaneously with the UART without a patched kernel.
From where I stand I am fairly confident in a documentation error, however I do not have access to the hardware required to prove this. Can someone at ASpeed look at the HDL and verify or correct the documentation? We have already caught one documentation error relating to COND2 and DVO, and verified that one in hardware.
Thank you!
^ permalink raw reply
* [PATCH 7/7] media: aspeed: refine interrupt handling logic
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-1-jae.hyun.yoo@linux.intel.com>
There are cases that interrupt bits are cleared by a 500ms delayed
work which causes unnecessary irq calls. Also, the current
interrupt handler returns IRQ_HANDLED always but it should return
IRQ_NONE if there is any unhandled interrupt. So this commit
refines the interrupt handling logic to fix these issues.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/media/platform/aspeed-video.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 8d0bb395e46d..98944a911998 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -488,6 +488,7 @@ static void aspeed_video_off(struct aspeed_video *video)
/* Disable interrupts */
aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
+ aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
/* Turn off the relevant clocks */
clk_disable(video->vclk);
@@ -556,7 +557,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
VE_INTERRUPT_MODE_DETECT, 0);
aspeed_video_write(video, VE_INTERRUPT_STATUS,
VE_INTERRUPT_MODE_DETECT);
-
+ sts &= ~VE_INTERRUPT_MODE_DETECT;
set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
wake_up_interruptible_all(&video->wait);
} else {
@@ -601,12 +602,12 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
VE_INTERRUPT_COMP_COMPLETE, 0);
aspeed_video_write(video, VE_INTERRUPT_STATUS,
VE_INTERRUPT_COMP_COMPLETE);
-
+ sts &= ~VE_INTERRUPT_COMP_COMPLETE;
if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
aspeed_video_start_frame(video);
}
- return IRQ_HANDLED;
+ return sts ? IRQ_NONE : IRQ_HANDLED;
}
static void aspeed_video_check_and_set_polarity(struct aspeed_video *video)
--
2.21.0
^ permalink raw reply related
* [PATCH 6/7] media: aspeed: remove checking of VE_INTERRUPT_CAPTURE_COMPLETE
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-1-jae.hyun.yoo@linux.intel.com>
VE_INTERRUPT_CAPTURE_COMPLETE and VE_INTERRUPT_COMP_COMPLETE are
not set at the same time but the current interrupt handling
mechanism of this driver doesn't clear the interrupt flag until
both two are set, and this behavior causes unnecessary interrupt
handler calls. In fact, this driver provides JPEG format only so
taking care of the VE_INTERRUPT_COMP_COMPLETE is enough for getting
compressed image frame so this commit gets rid of the
VE_INTERRUPT_CAPTURE_COMPLETE checking logic to simplify the logic.
Handling of VE_INTERRUPT_CAPTURE_COMPLETE could be added back later
when it's actually needed.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/media/platform/aspeed-video.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 6dc66d6e8b8b..8d0bb395e46d 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -463,8 +463,7 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
aspeed_video_write(video, VE_COMP_ADDR, addr);
aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
- VE_INTERRUPT_COMP_COMPLETE |
- VE_INTERRUPT_CAPTURE_COMPLETE);
+ VE_INTERRUPT_COMP_COMPLETE);
aspeed_video_update(video, VE_SEQ_CTRL, 0,
VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
@@ -570,8 +569,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
}
}
- if ((sts & VE_INTERRUPT_COMP_COMPLETE) &&
- (sts & VE_INTERRUPT_CAPTURE_COMPLETE)) {
+ if (sts & VE_INTERRUPT_COMP_COMPLETE) {
struct aspeed_video_buffer *buf;
u32 frame_size = aspeed_video_read(video,
VE_OFFSET_COMP_STREAM);
@@ -600,11 +598,9 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
VE_SEQ_CTRL_FORCE_IDLE |
VE_SEQ_CTRL_TRIG_COMP, 0);
aspeed_video_update(video, VE_INTERRUPT_CTRL,
- VE_INTERRUPT_COMP_COMPLETE |
- VE_INTERRUPT_CAPTURE_COMPLETE, 0);
+ VE_INTERRUPT_COMP_COMPLETE, 0);
aspeed_video_write(video, VE_INTERRUPT_STATUS,
- VE_INTERRUPT_COMP_COMPLETE |
- VE_INTERRUPT_CAPTURE_COMPLETE);
+ VE_INTERRUPT_COMP_COMPLETE);
if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
aspeed_video_start_frame(video);
--
2.21.0
^ permalink raw reply related
* [PATCH 5/7] media: aspeed: reduce noisy log printing outs
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-1-jae.hyun.yoo@linux.intel.com>
Currently, this driver prints out too much log messages when a
mode change happens, video turned off by screen saver and etc.
Actually, all cases are reported to user space properly. Also,
these are not critical errors but recoverable things, so this
commit changes the log level of some noisy printing outs.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index d88e8d238c49..6dc66d6e8b8b 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -441,7 +441,7 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
if (!(seq_ctrl & VE_SEQ_CTRL_COMP_BUSY) ||
!(seq_ctrl & VE_SEQ_CTRL_CAP_BUSY)) {
- dev_err(video->dev, "Engine busy; don't start frame\n");
+ dev_dbg(video->dev, "Engine busy; don't start frame\n");
return -EBUSY;
}
@@ -771,7 +771,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
res_check(video),
MODE_DETECT_TIMEOUT);
if (!rc) {
- dev_err(video->dev, "Timed out; first mode detect\n");
+ dev_dbg(video->dev, "Timed out; first mode detect\n");
clear_bit(VIDEO_RES_DETECT, &video->flags);
return;
}
@@ -789,7 +789,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
MODE_DETECT_TIMEOUT);
clear_bit(VIDEO_RES_DETECT, &video->flags);
if (!rc) {
- dev_err(video->dev, "Timed out; second mode detect\n");
+ dev_dbg(video->dev, "Timed out; second mode detect\n");
return;
}
@@ -823,7 +823,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
} while (invalid_resolution && (tries++ < INVALID_RESOLUTION_RETRIES));
if (invalid_resolution) {
- dev_err(video->dev, "Invalid resolution detected\n");
+ dev_dbg(video->dev, "Invalid resolution detected\n");
return;
}
@@ -1471,7 +1471,7 @@ static void aspeed_video_stop_streaming(struct vb2_queue *q)
!test_bit(VIDEO_FRAME_INPRG, &video->flags),
STOP_TIMEOUT);
if (!rc) {
- dev_err(video->dev, "Timed out when stopping streaming\n");
+ dev_dbg(video->dev, "Timed out when stopping streaming\n");
/*
* Need to force stop any DMA and try and get HW into a good
--
2.21.0
^ permalink raw reply related
* [PATCH 4/7] media: aspeed: remove IRQF_SHARED flag
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-1-jae.hyun.yoo@linux.intel.com>
Video Engine has a dedicated interrupt line so this driver doesn't
need to use IRQF_SHARED flag so remove it. Also, it'd be good for
following what Thomas recommended in the IRQF_ONESHOT support
patch like below:
"Note that for now IRQF_ONESHOT cannot be used with IRQF_SHARED to
avoid complex accounting mechanisms."
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index e1bdafeed007..d88e8d238c49 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -1616,8 +1616,7 @@ static int aspeed_video_init(struct aspeed_video *video)
}
rc = devm_request_threaded_irq(dev, irq, NULL, aspeed_video_irq,
- IRQF_ONESHOT | IRQF_SHARED, DEVICE_NAME,
- video);
+ IRQF_ONESHOT, DEVICE_NAME, video);
if (rc < 0) {
dev_err(dev, "Unable to request IRQ %d\n", irq);
return rc;
--
2.21.0
^ permalink raw reply related
* [PATCH 3/7] media: aspeed: change irq to threaded irq
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-1-jae.hyun.yoo@linux.intel.com>
Differently fron other Aspeed drivers, this driver calls clock
control APIs in interrupt context. Since ECLK is coupled with a
reset bit in clk-aspeed module, aspeed_clk_enable will make 10ms of
busy waiting delay for triggering the reset and it will eventually
disturb other drivers' interrupt handling. To fix this issue, this
commit changes this driver's irq to threaded irq so that the delay
can be happened in a thread context.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 8f4cac41da14..e1bdafeed007 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -1615,8 +1615,9 @@ static int aspeed_video_init(struct aspeed_video *video)
return -ENODEV;
}
- rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED,
- DEVICE_NAME, video);
+ rc = devm_request_threaded_irq(dev, irq, NULL, aspeed_video_irq,
+ IRQF_ONESHOT | IRQF_SHARED, DEVICE_NAME,
+ video);
if (rc < 0) {
dev_err(dev, "Unable to request IRQ %d\n", irq);
return rc;
--
2.21.0
^ permalink raw reply related
* [PATCH 2/7] media: aspeed: refine clock control logic
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-1-jae.hyun.yoo@linux.intel.com>
Currently, this driver calls clk_prepare and clk_unprepare from
interrupt context too but these should be called from sleepable
context only. To fix this issue, this commit splits out
clk_enable/disable and clk_prepare/unprepare, and it places
clk_prepare/unprepare calls into the module probe/remove function.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 38 ++++++++++++++++++++-------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 2dac6d20b180..8f4cac41da14 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -491,8 +491,8 @@ static void aspeed_video_off(struct aspeed_video *video)
aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
/* Turn off the relevant clocks */
- clk_disable_unprepare(video->vclk);
- clk_disable_unprepare(video->eclk);
+ clk_disable(video->vclk);
+ clk_disable(video->eclk);
clear_bit(VIDEO_CLOCKS_ON, &video->flags);
}
@@ -503,8 +503,8 @@ static void aspeed_video_on(struct aspeed_video *video)
return;
/* Turn on the relevant clocks */
- clk_prepare_enable(video->eclk);
- clk_prepare_enable(video->vclk);
+ clk_enable(video->eclk);
+ clk_enable(video->vclk);
set_bit(VIDEO_CLOCKS_ON, &video->flags);
}
@@ -1628,12 +1628,21 @@ static int aspeed_video_init(struct aspeed_video *video)
return PTR_ERR(video->eclk);
}
+ rc = clk_prepare(video->eclk);
+ if (rc)
+ return rc;
+
video->vclk = devm_clk_get(dev, "vclk");
if (IS_ERR(video->vclk)) {
dev_err(dev, "Unable to get VCLK\n");
- return PTR_ERR(video->vclk);
+ rc = PTR_ERR(video->vclk);
+ goto err_unprepare_eclk;
}
+ rc = clk_prepare(video->vclk);
+ if (rc)
+ goto err_unprepare_eclk;
+
rc = of_reserved_mem_device_init(dev);
if (rc) {
dev_err(dev, "Unable to reserve memory\n");
@@ -1643,20 +1652,26 @@ static int aspeed_video_init(struct aspeed_video *video)
rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
if (rc) {
dev_err(dev, "Failed to set DMA mask\n");
- of_reserved_mem_device_release(dev);
- return rc;
+ goto err_release_reserved_mem;
}
if (!aspeed_video_alloc_buf(video, &video->jpeg,
VE_JPEG_HEADER_SIZE)) {
dev_err(dev, "Failed to allocate DMA for JPEG header\n");
- of_reserved_mem_device_release(dev);
- return rc;
+ goto err_release_reserved_mem;
}
aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
return 0;
+
+err_release_reserved_mem:
+ of_reserved_mem_device_release(dev);
+ clk_unprepare(video->vclk);
+err_unprepare_eclk:
+ clk_unprepare(video->eclk);
+
+ return rc;
}
static int aspeed_video_probe(struct platform_device *pdev)
@@ -1700,6 +1715,11 @@ static int aspeed_video_remove(struct platform_device *pdev)
struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
struct aspeed_video *video = to_aspeed_video(v4l2_dev);
+ aspeed_video_off(video);
+
+ clk_unprepare(video->vclk);
+ clk_unprepare(video->eclk);
+
video_unregister_device(&video->vdev);
vb2_queue_release(&video->queue);
--
2.21.0
^ permalink raw reply related
* [PATCH 1/7] media: aspeed: fix a kernel warning on clk control
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502191317.29698-1-jae.hyun.yoo@linux.intel.com>
Video engine clock control functions in the Aspeed video engine
driver are being called from multiple context without any
protection so video clocks can be double disabled and eventually
it causes a kernel warning with stack dump printing out like below:
[ 515.540498] ------------[ cut here ]------------
[ 515.545174] WARNING: CPU: 0 PID: 1310 at drivers/clk/clk.c:684 clk_core_unprepare+0x13c/0x170
[ 515.553806] vclk-gate already unprepared
[ 515.557841] CPU: 0 PID: 1310 Comm: obmc-ikvm Tainted: G W 5.0.6-df66fbc97853fbba90a0bfa44de32f3d5f7602b4 #1
[ 515.568973] Hardware name: Generic DT based system
[ 515.573777] Backtrace:
[ 515.576272] [<80107cdc>] (dump_backtrace) from [<80107f10>] (show_stack+0x20/0x24)
[ 515.583930] r7:803a5614 r6:00000009 r5:00000000 r4:9d88fe1c
[ 515.589712] [<80107ef0>] (show_stack) from [<80690184>] (dump_stack+0x20/0x28)
[ 515.597053] [<80690164>] (dump_stack) from [<80116044>] (__warn.part.3+0xb4/0xdc)
[ 515.604557] [<80115f90>] (__warn.part.3) from [<801160d8>] (warn_slowpath_fmt+0x6c/0x90)
[ 515.612734] r6:000002ac r5:8080befc r4:80a07008
[ 515.617463] [<80116070>] (warn_slowpath_fmt) from [<803a5614>] (clk_core_unprepare+0x13c/0x170)
[ 515.626167] r3:8080cdf4 r2:8080bfc0
[ 515.629834] r7:98d682a8 r6:9d8a9200 r5:9e5151a0 r4:97abd620
[ 515.635530] [<803a54d8>] (clk_core_unprepare) from [<803a76a4>] (clk_unprepare+0x34/0x3c)
[ 515.643812] r5:9e5151a0 r4:97abd620
[ 515.647529] [<803a7670>] (clk_unprepare) from [<804f36ec>] (aspeed_video_off+0x38/0x50)
[ 515.655539] r5:9e5151a0 r4:9e504000
[ 515.659242] [<804f36b4>] (aspeed_video_off) from [<804f4358>] (aspeed_video_release+0x90/0x114)
[ 515.668036] r5:9e5044b0 r4:9e504000
[ 515.671643] [<804f42c8>] (aspeed_video_release) from [<804d302c>] (v4l2_release+0xd4/0xe8)
[ 515.679999] r7:98d682a8 r6:9d087810 r5:9d8a9200 r4:9e504318
[ 515.685695] [<804d2f58>] (v4l2_release) from [<80236454>] (__fput+0x98/0x1c4)
[ 515.692914] r5:9e51b608 r4:9d8a9200
[ 515.696597] [<802363bc>] (__fput) from [<802365e8>] (____fput+0x18/0x1c)
[ 515.703315] r9:80a0700c r8:801011e4 r7:00000000 r6:80a64b9c r5:9d8e35a0 r4:9d8e38dc
[ 515.711167] [<802365d0>] (____fput) from [<80131ca4>] (task_work_run+0x7c/0xa0)
[ 515.718596] [<80131c28>] (task_work_run) from [<80106884>] (do_work_pending+0x4a8/0x578)
[ 515.726777] r7:801011e4 r6:80a07008 r5:9d88ffb0 r4:ffffe000
[ 515.732466] [<801063dc>] (do_work_pending) from [<8010106c>] (slow_work_pending+0xc/0x20)
[ 515.740727] Exception stack(0x9d88ffb0 to 0x9d88fff8)
[ 515.745840] ffa0: 00000000 76f18094 00000000 00000000
[ 515.754122] ffc0: 00000007 00176778 7eda4c20 00000006 00000000 00000000 48e20fa4 00000000
[ 515.762386] ffe0: 00000002 7eda4b08 00000000 48f91efc 80000010 00000007
[ 515.769097] r10:00000000 r9:9d88e000 r8:801011e4 r7:00000006 r6:7eda4c20 r5:00176778
[ 515.777006] r4:00000007
[ 515.779558] ---[ end trace 12c04aadef8afbbb ]---
[ 515.784176] ------------[ cut here ]------------
[ 515.788817] WARNING: CPU: 0 PID: 1310 at drivers/clk/clk.c:825 clk_core_disable+0x18c/0x204
[ 515.797161] eclk-gate already disabled
[ 515.800916] CPU: 0 PID: 1310 Comm: obmc-ikvm Tainted: G W 5.0.6-df66fbc97853fbba90a0bfa44de32f3d5f7602b4 #1
[ 515.811945] Hardware name: Generic DT based system
[ 515.816730] Backtrace:
[ 515.819210] [<80107cdc>] (dump_backtrace) from [<80107f10>] (show_stack+0x20/0x24)
[ 515.826782] r7:803a5900 r6:00000009 r5:00000000 r4:9d88fe04
[ 515.832454] [<80107ef0>] (show_stack) from [<80690184>] (dump_stack+0x20/0x28)
[ 515.839687] [<80690164>] (dump_stack) from [<80116044>] (__warn.part.3+0xb4/0xdc)
[ 515.847170] [<80115f90>] (__warn.part.3) from [<801160d8>] (warn_slowpath_fmt+0x6c/0x90)
[ 515.855247] r6:00000339 r5:8080befc r4:80a07008
[ 515.859868] [<80116070>] (warn_slowpath_fmt) from [<803a5900>] (clk_core_disable+0x18c/0x204)
[ 515.868385] r3:8080cdd0 r2:8080c00c
[ 515.871957] r7:98d682a8 r6:9d8a9200 r5:97abd560 r4:97abd560
[ 515.877615] [<803a5774>] (clk_core_disable) from [<803a59a0>] (clk_core_disable_lock+0x28/0x34)
[ 515.886301] r7:98d682a8 r6:9d8a9200 r5:97abd560 r4:a0000013
[ 515.891960] [<803a5978>] (clk_core_disable_lock) from [<803a7714>] (clk_disable+0x2c/0x30)
[ 515.900216] r5:9e5151a0 r4:9e515f60
[ 515.903816] [<803a76e8>] (clk_disable) from [<804f36f8>] (aspeed_video_off+0x44/0x50)
[ 515.911656] [<804f36b4>] (aspeed_video_off) from [<804f4358>] (aspeed_video_release+0x90/0x114)
[ 515.920341] r5:9e5044b0 r4:9e504000
[ 515.923921] [<804f42c8>] (aspeed_video_release) from [<804d302c>] (v4l2_release+0xd4/0xe8)
[ 515.932184] r7:98d682a8 r6:9d087810 r5:9d8a9200 r4:9e504318
[ 515.937851] [<804d2f58>] (v4l2_release) from [<80236454>] (__fput+0x98/0x1c4)
[ 515.944980] r5:9e51b608 r4:9d8a9200
[ 515.948559] [<802363bc>] (__fput) from [<802365e8>] (____fput+0x18/0x1c)
[ 515.955257] r9:80a0700c r8:801011e4 r7:00000000 r6:80a64b9c r5:9d8e35a0 r4:9d8e38dc
[ 515.963008] [<802365d0>] (____fput) from [<80131ca4>] (task_work_run+0x7c/0xa0)
[ 515.970333] [<80131c28>] (task_work_run) from [<80106884>] (do_work_pending+0x4a8/0x578)
[ 515.978421] r7:801011e4 r6:80a07008 r5:9d88ffb0 r4:ffffe000
[ 515.984086] [<801063dc>] (do_work_pending) from [<8010106c>] (slow_work_pending+0xc/0x20)
[ 515.992247] Exception stack(0x9d88ffb0 to 0x9d88fff8)
[ 515.997296] ffa0: 00000000 76f18094 00000000 00000000
[ 516.005473] ffc0: 00000007 00176778 7eda4c20 00000006 00000000 00000000 48e20fa4 00000000
[ 516.013642] ffe0: 00000002 7eda4b08 00000000 48f91efc 80000010 00000007
[ 516.020257] r10:00000000 r9:9d88e000 r8:801011e4 r7:00000006 r6:7eda4c20 r5:00176778
[ 516.028072] r4:00000007
[ 516.030606] ---[ end trace 12c04aadef8afbbc ]---
To prevent this issue, this commit adds spinlock protection and
clock status checking logic into the Aspeed video engine driver.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Eddie James <eajames@linux.ibm.com>
---
drivers/media/platform/aspeed-video.c | 32 ++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 55c55a68b016..2dac6d20b180 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -187,6 +187,7 @@ enum {
VIDEO_STREAMING,
VIDEO_FRAME_INPRG,
VIDEO_STOPPED,
+ VIDEO_CLOCKS_ON,
};
struct aspeed_video_addr {
@@ -483,19 +484,29 @@ static void aspeed_video_enable_mode_detect(struct aspeed_video *video)
static void aspeed_video_off(struct aspeed_video *video)
{
+ if (!test_bit(VIDEO_CLOCKS_ON, &video->flags))
+ return;
+
/* Disable interrupts */
aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
/* Turn off the relevant clocks */
clk_disable_unprepare(video->vclk);
clk_disable_unprepare(video->eclk);
+
+ clear_bit(VIDEO_CLOCKS_ON, &video->flags);
}
static void aspeed_video_on(struct aspeed_video *video)
{
+ if (test_bit(VIDEO_CLOCKS_ON, &video->flags))
+ return;
+
/* Turn on the relevant clocks */
clk_prepare_enable(video->eclk);
clk_prepare_enable(video->vclk);
+
+ set_bit(VIDEO_CLOCKS_ON, &video->flags);
}
static void aspeed_video_bufs_done(struct aspeed_video *video,
@@ -513,12 +524,14 @@ static void aspeed_video_bufs_done(struct aspeed_video *video,
static void aspeed_video_irq_res_change(struct aspeed_video *video)
{
+ spin_lock(&video->lock);
dev_dbg(video->dev, "Resolution changed; resetting\n");
set_bit(VIDEO_RES_CHANGE, &video->flags);
clear_bit(VIDEO_FRAME_INPRG, &video->flags);
aspeed_video_off(video);
+ spin_unlock(&video->lock);
aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
schedule_delayed_work(&video->res_work, RESOLUTION_CHANGE_DELAY);
@@ -938,9 +951,13 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
static void aspeed_video_start(struct aspeed_video *video)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&video->lock, flags);
aspeed_video_on(video);
aspeed_video_init_regs(video);
+ spin_unlock_irqrestore(&video->lock, flags);
/* Resolution set to 640x480 if no signal found */
aspeed_video_get_resolution(video);
@@ -956,6 +973,9 @@ static void aspeed_video_start(struct aspeed_video *video)
static void aspeed_video_stop(struct aspeed_video *video)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&video->lock, flags);
set_bit(VIDEO_STOPPED, &video->flags);
cancel_delayed_work_sync(&video->res_work);
@@ -969,6 +989,7 @@ static void aspeed_video_stop(struct aspeed_video *video)
video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
video->flags = 0;
+ spin_unlock_irqrestore(&video->lock, flags);
}
static int aspeed_video_querycap(struct file *file, void *fh,
@@ -1306,16 +1327,21 @@ static void aspeed_video_resolution_work(struct work_struct *work)
struct delayed_work *dwork = to_delayed_work(work);
struct aspeed_video *video = container_of(dwork, struct aspeed_video,
res_work);
- u32 input_status = video->v4l2_input_status;
+ unsigned long flags;
+ u32 input_status;
+ spin_lock_irqsave(&video->lock, flags);
+ input_status = video->v4l2_input_status;
aspeed_video_on(video);
/* Exit early in case no clients remain */
- if (test_bit(VIDEO_STOPPED, &video->flags))
+ if (test_bit(VIDEO_STOPPED, &video->flags)) {
+ spin_unlock_irqrestore(&video->lock, flags);
goto done;
+ }
aspeed_video_init_regs(video);
-
+ spin_unlock_irqrestore(&video->lock, flags);
aspeed_video_get_resolution(video);
if (video->detected_timings.width != video->active_timings.width ||
--
2.21.0
^ permalink raw reply related
* [PATCH 0/7] Improve stability of Aspeed video engine driver
From: Jae Hyun Yoo @ 2019-05-02 19:13 UTC (permalink / raw)
To: linux-aspeed
This patch series improves stability of Aspeed video engine driver by fixing
clock control and irq handling logic in the driver.
This series should be applied on top of:
https://www.spinics.net/lists/linux-media/msg150193.html
Jae Hyun Yoo (7):
media: aspeed: fix a kernel warning on clk control
media: aspeed: refine clock control logic
media: aspeed: change irq to threaded irq
media: aspeed: remove IRQF_SHARED flag
media: aspeed: reduce noisy log printing outs
media: aspeed: remove checking of VE_INTERRUPT_CAPTURE_COMPLETE
media: aspeed: refine interrupt handling logic
drivers/media/platform/aspeed-video.c | 103 ++++++++++++++++++--------
1 file changed, 73 insertions(+), 30 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH] misc: aspeed-lpc-ctrl: Correct return values
From: Andrew Jeffery @ 2019-05-02 6:49 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190502064021.GA14911@kroah.com>
On Thu, 2 May 2019, at 16:10, Greg Kroah-Hartman wrote:
> On Wed, May 01, 2019 at 03:38:36PM -0700, Vijay Khemka wrote:
> > Corrected some of return values with appropriate meanings.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> > index 332210e06e98..97ae341109d5 100644
> > --- a/drivers/misc/aspeed-lpc-ctrl.c
> > +++ b/drivers/misc/aspeed-lpc-ctrl.c
> > @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> > unsigned long param)
> > {
> > struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> > - struct device *dev = file->private_data;
> > void __user *p = (void __user *)param;
> > struct aspeed_lpc_ctrl_mapping map;
> > u32 addr;
>
> This change is not reflected in your changelog text :(
>
> Please fix up, or break this up into multiple patches.
The return value fixes should also be squashed into the patch that introduced those lines
given it hasn't yet been applied.
Further, IIRC I previously suggested removing the dev_err()s entirely, not just switching
them to pr_err(). Returning an error code is enough IMO, there's no need to pollute the
kernel logs with application-level errors. Or make them dev_dbg().
Andrew
>
> greg k-h
>
^ permalink raw reply
* Re: [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
From: Andrew Jeffery @ 2019-05-02 6:40 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1991472336.3446950.1556778801333.JavaMail.zimbra@raptorengineeringinc.com>
On Thu, 2 May 2019, at 16:03, Timothy Pearson wrote:
>
>
> ----- Original Message -----
> > From: "Andrew Jeffery" <andrew@aj.id.au>
> > To: "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed" <linux-aspeed@lists.ozlabs.org>, "Ryan Chen"
> > <ryan_chen@aspeedtech.com>
> > Sent: Thursday, May 2, 2019 12:51:00 AM
> > Subject: Re: [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
>
> > On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
> >> There appears to be a significant error in the pinmux table starting on
> >> page 127 of the AST2500 datasheet v1.6. Specifically, the COND2 (DVO)
> >> requirement is incorrectly applied to multiple digital video input (DVI)
> >> muxed pins, and no DVI-specific condition is provided. This results in
> >> the serial devices incorrectly overriding the DVO pinmuxes and disabling
> >> the DVO pins.
> >>
> >> Create a new condition code (COND6) for DVI enable, and update the most
> >> seriously affected pins to use the new condition code.
> >>
> >> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> >> ---
> >> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 17 +++++++++--------
> >> 1 file changed, 9 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> >> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> >> index 6f357a11e89a..676f90d3c5f3 100644
> >> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> >> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> >> @@ -29,6 +29,7 @@
> >>
> >> #define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
> >> #define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
> >> +#define COND6 { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 0, 0 }
> >>
> >> /* LHCR0 is offset from the end of the H8S/2168-compatible registers */
> >> #define LHCR0 0x20
> >> @@ -660,8 +661,8 @@ SSSF_PIN_DECL(T2, GPIOL0, NCTS1, SIG_DESC_SET(SCU84, 16));
> >>
> >> #define T1 89
> >> #define T1_DESC SIG_DESC_SET(SCU84, 17)
> >> -SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND2);
> >> -SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND2);
> >> +SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND6);
> >> +SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND6);
> >
> > I feel like you didn't test this patch, because VPI_24_RSVD_DESC (the DVI
> > condition)
> > requires SCU90[5]=0b1, but your introduction of COND6 requires SCU90[5:4]=0b00
> > for
> > the mux configuration to succeed. This can't work - bit 5 of SCU90 can not
> > simultaneously take the values 1 and 0.
>
> That's correct -- I don't have hardware that supports DVI available to
> test with.
Okay. In that case I'm not prepared to ACK changes here, much less changes with
that fail in this way. The current implementation at least follows what is dictated by
the programming guide and is at least correct in theory.
As Ryan is not confident there are no negative side-effects to not following the
configuration dictated by the programming guide, changes like this have a real
up-hill battle on their hands.
Cheers,
Andrew
>
> > Ryan: Can we just drop the COND2 requirement for function 2 of balls T1, U2, P4
> > and P3?
> > I think that gets us where we need to be?
> >
> > Cheers,
> >
> > Andrew
> >
> >> MS_PIN_DECL(T1, GPIOL1, VPIDE, NDCD1);
> >> FUNC_GROUP_DECL(NDCD1, T1);
> >>
> >> @@ -674,22 +675,22 @@ FUNC_GROUP_DECL(NDSR1, U1);
> >>
> >> #define U2 91
> >> #define U2_DESC SIG_DESC_SET(SCU84, 19)
> >> -SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC, COND2);
> >> -SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND2);
> >> +SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC, COND6);
> >> +SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND6);
> >> MS_PIN_DECL(U2, GPIOL3, VPIHS, NRI1);
> >> FUNC_GROUP_DECL(NRI1, U2);
> >>
> >> #define P4 92
> >> #define P4_DESC SIG_DESC_SET(SCU84, 20)
> >> -SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC, COND2);
> >> -SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND2);
> >> +SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC, COND6);
> >> +SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND6);
> >> MS_PIN_DECL(P4, GPIOL4, VPIVS, NDTR1);
> >> FUNC_GROUP_DECL(NDTR1, P4);
> >>
> >> #define P3 93
> >> #define P3_DESC SIG_DESC_SET(SCU84, 21)
> >> -SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC, COND2);
> >> -SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND2);
> >> +SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC, COND6);
> >> +SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND6);
> >> MS_PIN_DECL(P3, GPIOL5, VPICLK, NRTS1);
> >> FUNC_GROUP_DECL(NRTS1, P3);
> >>
> >> --
> >> 2.11.0
>
^ permalink raw reply
* [PATCH] misc: aspeed-lpc-ctrl: Correct return values
From: Greg Kroah-Hartman @ 2019-05-02 6:40 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190501223836.1670096-1-vijaykhemka@fb.com>
On Wed, May 01, 2019 at 03:38:36PM -0700, Vijay Khemka wrote:
> Corrected some of return values with appropriate meanings.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> index 332210e06e98..97ae341109d5 100644
> --- a/drivers/misc/aspeed-lpc-ctrl.c
> +++ b/drivers/misc/aspeed-lpc-ctrl.c
> @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> unsigned long param)
> {
> struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> - struct device *dev = file->private_data;
> void __user *p = (void __user *)param;
> struct aspeed_lpc_ctrl_mapping map;
> u32 addr;
This change is not reflected in your changelog text :(
Please fix up, or break this up into multiple patches.
greg k-h
^ permalink raw reply
* [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
From: Timothy Pearson @ 2019-05-02 6:33 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <128da9c8-d138-47b9-b323-b845bd93ca2f@www.fastmail.com>
----- Original Message -----
> From: "Andrew Jeffery" <andrew@aj.id.au>
> To: "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed" <linux-aspeed@lists.ozlabs.org>, "Ryan Chen"
> <ryan_chen@aspeedtech.com>
> Sent: Thursday, May 2, 2019 12:51:00 AM
> Subject: Re: [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
> On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
>> There appears to be a significant error in the pinmux table starting on
>> page 127 of the AST2500 datasheet v1.6. Specifically, the COND2 (DVO)
>> requirement is incorrectly applied to multiple digital video input (DVI)
>> muxed pins, and no DVI-specific condition is provided. This results in
>> the serial devices incorrectly overriding the DVO pinmuxes and disabling
>> the DVO pins.
>>
>> Create a new condition code (COND6) for DVI enable, and update the most
>> seriously affected pins to use the new condition code.
>>
>> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
>> ---
>> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 17 +++++++++--------
>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> index 6f357a11e89a..676f90d3c5f3 100644
>> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
>> @@ -29,6 +29,7 @@
>>
>> #define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
>> #define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
>> +#define COND6 { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 0, 0 }
>>
>> /* LHCR0 is offset from the end of the H8S/2168-compatible registers */
>> #define LHCR0 0x20
>> @@ -660,8 +661,8 @@ SSSF_PIN_DECL(T2, GPIOL0, NCTS1, SIG_DESC_SET(SCU84, 16));
>>
>> #define T1 89
>> #define T1_DESC SIG_DESC_SET(SCU84, 17)
>> -SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND2);
>> -SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND2);
>> +SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND6);
>> +SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND6);
>
> I feel like you didn't test this patch, because VPI_24_RSVD_DESC (the DVI
> condition)
> requires SCU90[5]=0b1, but your introduction of COND6 requires SCU90[5:4]=0b00
> for
> the mux configuration to succeed. This can't work - bit 5 of SCU90 can not
> simultaneously take the values 1 and 0.
That's correct -- I don't have hardware that supports DVI available to test with.
> Ryan: Can we just drop the COND2 requirement for function 2 of balls T1, U2, P4
> and P3?
> I think that gets us where we need to be?
>
> Cheers,
>
> Andrew
>
>> MS_PIN_DECL(T1, GPIOL1, VPIDE, NDCD1);
>> FUNC_GROUP_DECL(NDCD1, T1);
>>
>> @@ -674,22 +675,22 @@ FUNC_GROUP_DECL(NDSR1, U1);
>>
>> #define U2 91
>> #define U2_DESC SIG_DESC_SET(SCU84, 19)
>> -SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC, COND2);
>> -SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND2);
>> +SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC, COND6);
>> +SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND6);
>> MS_PIN_DECL(U2, GPIOL3, VPIHS, NRI1);
>> FUNC_GROUP_DECL(NRI1, U2);
>>
>> #define P4 92
>> #define P4_DESC SIG_DESC_SET(SCU84, 20)
>> -SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC, COND2);
>> -SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND2);
>> +SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC, COND6);
>> +SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND6);
>> MS_PIN_DECL(P4, GPIOL4, VPIVS, NDTR1);
>> FUNC_GROUP_DECL(NDTR1, P4);
>>
>> #define P3 93
>> #define P3_DESC SIG_DESC_SET(SCU84, 21)
>> -SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC, COND2);
>> -SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND2);
>> +SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC, COND6);
>> +SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND6);
>> MS_PIN_DECL(P3, GPIOL5, VPICLK, NRTS1);
>> FUNC_GROUP_DECL(NRTS1, P3);
>>
>> --
>> 2.11.0
^ permalink raw reply
* [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
From: Ryan Chen @ 2019-05-02 6:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <128da9c8-d138-47b9-b323-b845bd93ca2f@www.fastmail.com>
> There appears to be a significant error in the pinmux table starting
> on page 127 of the AST2500 datasheet v1.6. Specifically, the COND2
> (DVO) requirement is incorrectly applied to multiple digital video
> input (DVI) muxed pins, and no DVI-specific condition is provided.
> This results in the serial devices incorrectly overriding the DVO
> pinmuxes and disabling the DVO pins.
>
> Create a new condition code (COND6) for DVI enable, and update the
> most seriously affected pins to use the new condition code.
>
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> ---
> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> index 6f357a11e89a..676f90d3c5f3 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> @@ -29,6 +29,7 @@
>
> #define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
> #define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
> +#define COND6 { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 0, 0 }
>
> /* LHCR0 is offset from the end of the H8S/2168-compatible registers */
> #define LHCR0 0x20
> @@ -660,8 +661,8 @@ SSSF_PIN_DECL(T2, GPIOL0, NCTS1,
> SIG_DESC_SET(SCU84, 16));
>
> #define T1 89
> #define T1_DESC SIG_DESC_SET(SCU84, 17)
> -SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC,
> COND2); -SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC,
> +COND6); SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND6);
>>I feel like you didn't test this patch, because VPI_24_RSVD_DESC (the DVI condition) requires SCU90[5]=0b1, but your >>introduction of COND6 requires SCU90[5:4]=0b00 for the mux configuration to succeed. This can't work - bit 5 of SCU90 can not >>simultaneously take the values 1 and 0.
>>Ryan: Can we just drop the COND2 requirement for function 2 of balls T1, U2, P4 and P3?
>>I think that gets us where we need to be?
No, that will have some impact, we don't know.
> MS_PIN_DECL(T1, GPIOL1, VPIDE, NDCD1); FUNC_GROUP_DECL(NDCD1, T1);
>
> @@ -674,22 +675,22 @@ FUNC_GROUP_DECL(NDSR1, U1);
>
> #define U2 91
> #define U2_DESC SIG_DESC_SET(SCU84, 19)
> -SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC,
> COND2); -SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC,
> +COND6); SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND6);
> MS_PIN_DECL(U2, GPIOL3, VPIHS, NRI1); FUNC_GROUP_DECL(NRI1, U2);
>
> #define P4 92
> #define P4_DESC SIG_DESC_SET(SCU84, 20)
> -SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC,
> COND2); -SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC,
> +COND6); SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND6);
> MS_PIN_DECL(P4, GPIOL4, VPIVS, NDTR1); FUNC_GROUP_DECL(NDTR1, P4);
>
> #define P3 93
> #define P3_DESC SIG_DESC_SET(SCU84, 21)
> -SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC,
> COND2); -SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC,
> +COND6); SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND6);
> MS_PIN_DECL(P3, GPIOL5, VPICLK, NRTS1); FUNC_GROUP_DECL(NRTS1, P3);
>
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH 3/3] aspeed/pinctrl: Fix simultaneous DVO and serial outputs on AST2500 devices
From: Andrew Jeffery @ 2019-05-02 5:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <236762130.3394112.1556751009128.JavaMail.zimbra@raptorengineeringinc.com>
On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
> There appears to be a significant error in the pinmux table starting on
> page 127 of the AST2500 datasheet v1.6. Specifically, the COND2 (DVO)
> requirement is incorrectly applied to multiple digital video input (DVI)
> muxed pins, and no DVI-specific condition is provided. This results in
> the serial devices incorrectly overriding the DVO pinmuxes and disabling
> the DVO pins.
>
> Create a new condition code (COND6) for DVI enable, and update the most
> seriously affected pins to use the new condition code.
>
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> ---
> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> index 6f357a11e89a..676f90d3c5f3 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> @@ -29,6 +29,7 @@
>
> #define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
> #define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }
> +#define COND6 { ASPEED_IP_SCU, SCU90, GENMASK(5, 4), 0, 0 }
>
> /* LHCR0 is offset from the end of the H8S/2168-compatible registers */
> #define LHCR0 0x20
> @@ -660,8 +661,8 @@ SSSF_PIN_DECL(T2, GPIOL0, NCTS1, SIG_DESC_SET(SCU84, 16));
>
> #define T1 89
> #define T1_DESC SIG_DESC_SET(SCU84, 17)
> -SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND2);
> -SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPIDE, VPI24, VPI_24_RSVD_DESC, T1_DESC, COND6);
> +SIG_EXPR_LIST_DECL_SINGLE(NDCD1, NDCD1, T1_DESC, COND6);
I feel like you didn't test this patch, because VPI_24_RSVD_DESC (the DVI condition)
requires SCU90[5]=0b1, but your introduction of COND6 requires SCU90[5:4]=0b00 for
the mux configuration to succeed. This can't work - bit 5 of SCU90 can not
simultaneously take the values 1 and 0.
Ryan: Can we just drop the COND2 requirement for function 2 of balls T1, U2, P4 and P3?
I think that gets us where we need to be?
Cheers,
Andrew
> MS_PIN_DECL(T1, GPIOL1, VPIDE, NDCD1);
> FUNC_GROUP_DECL(NDCD1, T1);
>
> @@ -674,22 +675,22 @@ FUNC_GROUP_DECL(NDSR1, U1);
>
> #define U2 91
> #define U2_DESC SIG_DESC_SET(SCU84, 19)
> -SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC, COND2);
> -SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPIHS, VPI24, VPI_24_RSVD_DESC, U2_DESC, COND6);
> +SIG_EXPR_LIST_DECL_SINGLE(NRI1, NRI1, U2_DESC, COND6);
> MS_PIN_DECL(U2, GPIOL3, VPIHS, NRI1);
> FUNC_GROUP_DECL(NRI1, U2);
>
> #define P4 92
> #define P4_DESC SIG_DESC_SET(SCU84, 20)
> -SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC, COND2);
> -SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPIVS, VPI24, VPI_24_RSVD_DESC, P4_DESC, COND6);
> +SIG_EXPR_LIST_DECL_SINGLE(NDTR1, NDTR1, P4_DESC, COND6);
> MS_PIN_DECL(P4, GPIOL4, VPIVS, NDTR1);
> FUNC_GROUP_DECL(NDTR1, P4);
>
> #define P3 93
> #define P3_DESC SIG_DESC_SET(SCU84, 21)
> -SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC, COND2);
> -SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(VPICLK, VPI24, VPI_24_RSVD_DESC, P3_DESC, COND6);
> +SIG_EXPR_LIST_DECL_SINGLE(NRTS1, NRTS1, P3_DESC, COND6);
> MS_PIN_DECL(P3, GPIOL5, VPICLK, NRTS1);
> FUNC_GROUP_DECL(NRTS1, P3);
>
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM and DVO outputs on AST2500 devices
From: Andrew Jeffery @ 2019-05-02 5:36 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <08e21a55c7e746cda83694845c2b3429@TWMBX02.aspeed.com>
On Thu, 2 May 2019, at 14:29, Ryan Chen wrote:
> > ----- Original Message -----
> > > From: "Ryan Chen" <ryan_chen@aspeedtech.com>
> > > To: "Andrew Jeffery" <andrew@aj.id.au>, "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed"
> > > <linux-aspeed@lists.ozlabs.org>
> > > Cc: "Morris Mao" <morris_mao@aspeedtech.com>
> > > Sent: Wednesday, May 1, 2019 10:06:25 PM
> > > Subject: RE: [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 /
> > > PWM and DVO outputs on AST2500 devices
> >
> > >>On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
> > >> There appears to be a small error in the pinmux table on pages 130
> > >>and
> > >> 131 of the AST2500 datasheet v1.6. Specifically, the COND2
> > >>requirement used to mux the surrounding pins to DVI was
> > >>inadvertently replicated to pins V1, W1, V2, and W2 in the table,
> > >>which do not incorporate DVI functionality.
> > >>
> > >> As a result of this error, both serial TX lines and the PWM 0/1
> > >> outputs were overriding the VPO pinmux settings when VPO was
> > >> enabled in the pinmux hogs.
> > >>
> > >> This patch has been verified to function on Blackbird hardware.
> > >> Both serial TXD pins and PWM0/PWM1 were functionally tested with
> > >> SCU94[1:0] set to 0x1.
> > >
> > > Hello Tim.
> > >
> > > The AST2500 pwm0/1 configure need following condition, the
> > > SCU94[1:0] is 0x1, it should not work.
> > > Could you help confirm it?
> > >
> > > v2 : pwm 0 : scu88[0] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
> > > w2 : pwm 1 : scu88[1] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
> >
> > >>I can confirm that with SCU94[1:0] == 0x1 the PWM0 and PWM1 outputs work correctly -- this was tested on our Blackbird >>hardware. If you are reading from the datasheet, I suspect there are a few errors in it relating to the relatively rarely used DVO >>mux settings.
> >
> > Yes it can work after check with designer, if you don't enable the CRT
> > driver, it will work.
> > But for safety.
>
> >>What do you mean by "for safety"?
>
> Sorry, my point is if gfx driver also loaded, it will impact.
> But, It is ok for this modification.
Okay, that's enough confirmation for the moment.
> Due to pinctrl gfx pwm is separate
> driver setting.
>
>
> > You need also and with COND2 for pwm driver loaded.
>
> >>I'm confused here because it sounds like from Tim's experiment PWM0 / PWM1 work without the dependency on COND2 >>despite VPO being enabled, and the designer confirms as much, but we shouldn't do it?
>
> >>Regardless, in summary you're saying that for TXD1 and RXD1 the change to remove the dependence on COND2 is appropriate, but not for PWM0 and PWM1?
>
> >
> >
> >
>
^ permalink raw reply
* [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM and DVO outputs on AST2500 devices
From: Ryan Chen @ 2019-05-02 4:58 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <46d7b027-0f88-47e3-a132-e59b7640c867@www.fastmail.com>
> ----- Original Message -----
> > From: "Ryan Chen" <ryan_chen@aspeedtech.com>
> > To: "Andrew Jeffery" <andrew@aj.id.au>, "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed"
> > <linux-aspeed@lists.ozlabs.org>
> > Cc: "Morris Mao" <morris_mao@aspeedtech.com>
> > Sent: Wednesday, May 1, 2019 10:06:25 PM
> > Subject: RE: [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 /
> > PWM and DVO outputs on AST2500 devices
>
> >>On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
> >> There appears to be a small error in the pinmux table on pages 130
> >>and
> >> 131 of the AST2500 datasheet v1.6. Specifically, the COND2
> >>requirement used to mux the surrounding pins to DVI was
> >>inadvertently replicated to pins V1, W1, V2, and W2 in the table,
> >>which do not incorporate DVI functionality.
> >>
> >> As a result of this error, both serial TX lines and the PWM 0/1
> >> outputs were overriding the VPO pinmux settings when VPO was
> >> enabled in the pinmux hogs.
> >>
> >> This patch has been verified to function on Blackbird hardware.
> >> Both serial TXD pins and PWM0/PWM1 were functionally tested with
> >> SCU94[1:0] set to 0x1.
> >
> > Hello Tim.
> >
> > The AST2500 pwm0/1 configure need following condition, the
> > SCU94[1:0] is 0x1, it should not work.
> > Could you help confirm it?
> >
> > v2 : pwm 0 : scu88[0] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
> > w2 : pwm 1 : scu88[1] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
>
> >>I can confirm that with SCU94[1:0] == 0x1 the PWM0 and PWM1 outputs work correctly -- this was tested on our Blackbird >>hardware. If you are reading from the datasheet, I suspect there are a few errors in it relating to the relatively rarely used DVO >>mux settings.
>
> Yes it can work after check with designer, if you don't enable the CRT
> driver, it will work.
> But for safety.
>>What do you mean by "for safety"?
Sorry, my point is if gfx driver also loaded, it will impact.
But, It is ok for this modification. Due to pinctrl gfx pwm is separate driver setting.
> You need also and with COND2 for pwm driver loaded.
>>I'm confused here because it sounds like from Tim's experiment PWM0 / PWM1 work without the dependency on COND2 >>despite VPO being enabled, and the designer confirms as much, but we shouldn't do it?
>>Regardless, in summary you're saying that for TXD1 and RXD1 the change to remove the dependence on COND2 is appropriate, but not for PWM0 and PWM1?
>
>
>
^ permalink raw reply
* Re: [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM and DVO outputs on AST2500 devices
From: Andrew Jeffery @ 2019-05-02 4:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <921172cf50484d839bd30fa27ecf525e@TWMBX02.aspeed.com>
On Thu, 2 May 2019, at 13:17, Ryan Chen wrote:
> ----- Original Message -----
> > From: "Ryan Chen" <ryan_chen@aspeedtech.com>
> > To: "Andrew Jeffery" <andrew@aj.id.au>, "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed"
> > <linux-aspeed@lists.ozlabs.org>
> > Cc: "Morris Mao" <morris_mao@aspeedtech.com>
> > Sent: Wednesday, May 1, 2019 10:06:25 PM
> > Subject: RE: [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM
> > and DVO outputs on AST2500 devices
>
> >>On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
> >> There appears to be a small error in the pinmux table on pages 130
> >>and
> >> 131 of the AST2500 datasheet v1.6. Specifically, the COND2
> >>requirement used to mux the surrounding pins to DVI was inadvertently
> >>replicated to pins V1, W1, V2, and W2 in the table, which do not
> >>incorporate DVI functionality.
> >>
> >> As a result of this error, both serial TX lines and the PWM 0/1
> >> outputs were overriding the VPO pinmux settings when VPO was enabled
> >> in the pinmux hogs.
> >>
> >> This patch has been verified to function on Blackbird hardware. Both
> >> serial TXD pins and PWM0/PWM1 were functionally tested with
> >> SCU94[1:0] set to 0x1.
> >
> > Hello Tim.
> >
> > The AST2500 pwm0/1 configure need following condition, the SCU94[1:0]
> > is 0x1, it should not work.
> > Could you help confirm it?
> >
> > v2 : pwm 0 : scu88[0] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
> > w2 : pwm 1 : scu88[1] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
>
> >>I can confirm that with SCU94[1:0] == 0x1 the PWM0 and PWM1 outputs work correctly -- this was tested on our Blackbird >>hardware. If you are reading from the datasheet, I suspect there are a few errors in it relating to the relatively rarely used DVO >>mux settings.
>
> Yes it can work after check with designer, if you don't enable the CRT
> driver, it will work.
> But for safety.
What do you mean by "for safety"?
> You need also and with COND2 for pwm driver loaded.
I'm confused here because it sounds like from Tim's experiment PWM0
/ PWM1 work without the dependency on COND2 despite VPO being
enabled, and the designer confirms as much, but we shouldn't do it?
Regardless, in summary you're saying that for TXD1 and RXD1 the
change to remove the dependence on COND2 is appropriate, but
not for PWM0 and PWM1?
Andrew
>
>
>
^ permalink raw reply
* [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM and DVO outputs on AST2500 devices
From: Ryan Chen @ 2019-05-02 3:47 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <313461427.3426138.1556768056285.JavaMail.zimbra@raptorengineeringinc.com>
----- Original Message -----
> From: "Ryan Chen" <ryan_chen@aspeedtech.com>
> To: "Andrew Jeffery" <andrew@aj.id.au>, "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed"
> <linux-aspeed@lists.ozlabs.org>
> Cc: "Morris Mao" <morris_mao@aspeedtech.com>
> Sent: Wednesday, May 1, 2019 10:06:25 PM
> Subject: RE: [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM
> and DVO outputs on AST2500 devices
>>On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
>> There appears to be a small error in the pinmux table on pages 130
>>and
>> 131 of the AST2500 datasheet v1.6. Specifically, the COND2
>>requirement used to mux the surrounding pins to DVI was inadvertently
>>replicated to pins V1, W1, V2, and W2 in the table, which do not
>>incorporate DVI functionality.
>>
>> As a result of this error, both serial TX lines and the PWM 0/1
>> outputs were overriding the VPO pinmux settings when VPO was enabled
>> in the pinmux hogs.
>>
>> This patch has been verified to function on Blackbird hardware. Both
>> serial TXD pins and PWM0/PWM1 were functionally tested with
>> SCU94[1:0] set to 0x1.
>
> Hello Tim.
>
> The AST2500 pwm0/1 configure need following condition, the SCU94[1:0]
> is 0x1, it should not work.
> Could you help confirm it?
>
> v2 : pwm 0 : scu88[0] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
> w2 : pwm 1 : scu88[1] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
>>I can confirm that with SCU94[1:0] == 0x1 the PWM0 and PWM1 outputs work correctly -- this was tested on our Blackbird >>hardware. If you are reading from the datasheet, I suspect there are a few errors in it relating to the relatively rarely used DVO >>mux settings.
Yes it can work after check with designer, if you don't enable the CRT driver, it will work.
But for safety. You need also and with COND2 for pwm driver loaded.
^ permalink raw reply
* [PATCH] misc: aspeed-p2a-ctrl: fix mixed declarations
From: Andrew Jeffery @ 2019-05-02 3:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190426165655.218228-1-venture@google.com>
On Sat, 27 Apr 2019, at 02:27, Patrick Venture wrote:
> Fix up mixed declarations and code in aspeed_p2a_mmap.
>
> Tested: Verified the build had the error and that this patch resolved it
> and there were no other warnings or build errors associated with
> compilation of this driver.
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/misc/aspeed-p2a-ctrl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/aspeed-p2a-ctrl.c
> b/drivers/misc/aspeed-p2a-ctrl.c
> index 9736821972ef..b60fbeaffcbd 100644
> --- a/drivers/misc/aspeed-p2a-ctrl.c
> +++ b/drivers/misc/aspeed-p2a-ctrl.c
> @@ -100,6 +100,7 @@ static void aspeed_p2a_disable_bridge(struct
> aspeed_p2a_ctrl *p2a_ctrl)
> static int aspeed_p2a_mmap(struct file *file, struct vm_area_struct
> *vma)
> {
> unsigned long vsize;
> + pgprot_t prot;
> struct aspeed_p2a_user *priv = file->private_data;
> struct aspeed_p2a_ctrl *ctrl = priv->parent;
>
> @@ -107,7 +108,7 @@ static int aspeed_p2a_mmap(struct file *file,
> struct vm_area_struct *vma)
> return -EINVAL;
>
> vsize = vma->vm_end - vma->vm_start;
> - pgprot_t prot = vma->vm_page_prot;
> + prot = vma->vm_page_prot;
>
> if (vma->vm_pgoff + vsize > ctrl->mem_base + ctrl->mem_size)
> return -EINVAL;
> --
> 2.21.0.593.g511ec345e18-goog
>
>
^ permalink raw reply
* [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM and DVO outputs on AST2500 devices
From: Timothy Pearson @ 2019-05-02 3:34 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <d5a0e5a855144fd6bb65c569b37dedfa@TWMBX02.aspeed.com>
----- Original Message -----
> From: "Ryan Chen" <ryan_chen@aspeedtech.com>
> To: "Andrew Jeffery" <andrew@aj.id.au>, "Timothy Pearson" <tpearson@raptorengineering.com>, "linux-aspeed"
> <linux-aspeed@lists.ozlabs.org>
> Cc: "Morris Mao" <morris_mao@aspeedtech.com>
> Sent: Wednesday, May 1, 2019 10:06:25 PM
> Subject: RE: [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM and DVO outputs on AST2500 devices
>>On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
>> There appears to be a small error in the pinmux table on pages 130 and
>> 131 of the AST2500 datasheet v1.6. Specifically, the COND2
>> requirement used to mux the surrounding pins to DVI was inadvertently
>> replicated to pins V1, W1, V2, and W2 in the table, which do not
>> incorporate DVI functionality.
>>
>> As a result of this error, both serial TX lines and the PWM 0/1
>> outputs were overriding the VPO pinmux settings when VPO was enabled
>> in the pinmux hogs.
>>
>> This patch has been verified to function on Blackbird hardware. Both
>> serial TXD pins and PWM0/PWM1 were functionally tested with SCU94[1:0]
>> set to 0x1.
>
> Hello Tim.
>
> The AST2500 pwm0/1 configure need following condition, the SCU94[1:0] is 0x1, it
> should not work.
> Could you help confirm it?
>
> v2 : pwm 0 : scu88[0] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
> w2 : pwm 1 : scu88[1] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
I can confirm that with SCU94[1:0] == 0x1 the PWM0 and PWM1 outputs work correctly -- this was tested on our Blackbird hardware. If you are reading from the datasheet, I suspect there are a few errors in it relating to the relatively rarely used DVO mux settings.
^ permalink raw reply
* [PATCH 2/3] aspeed/pinctrl: Fix simultaneous RS-232 / PWM and DVO outputs on AST2500 devices
From: Ryan Chen @ 2019-05-02 3:06 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <f35bf045-48e4-432f-8239-29f8f6746158@www.fastmail.com>
>On Thu, 2 May 2019, at 08:20, Timothy Pearson wrote:
> There appears to be a small error in the pinmux table on pages 130 and
> 131 of the AST2500 datasheet v1.6. Specifically, the COND2
> requirement used to mux the surrounding pins to DVI was inadvertently
> replicated to pins V1, W1, V2, and W2 in the table, which do not
> incorporate DVI functionality.
>
> As a result of this error, both serial TX lines and the PWM 0/1
> outputs were overriding the VPO pinmux settings when VPO was enabled
> in the pinmux hogs.
>
> This patch has been verified to function on Blackbird hardware. Both
> serial TXD pins and PWM0/PWM1 were functionally tested with SCU94[1:0]
> set to 0x1.
Hello Tim.
The AST2500 pwm0/1 configure need following condition, the SCU94[1:0] is 0x1, it should not work.
Could you help confirm it?
v2 : pwm 0 : scu88[0] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
w2 : pwm 1 : scu88[1] = 1 & scu 94[1:0] = 0 & scu90[5] = 0
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> ---
> drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> index 187abd7693cf..6f357a11e89a 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
> @@ -696,14 +696,14 @@ FUNC_GROUP_DECL(NRTS1, P3); #define V1 94
> #define V1_DESC SIG_DESC_SET(SCU84, 22)
> SIG_EXPR_LIST_DECL_SINGLE(DASHV1, DASHV1, VPIRSVD_DESC, V1_DESC);
> -SIG_EXPR_LIST_DECL_SINGLE(TXD1, TXD1, V1_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(TXD1, TXD1, V1_DESC);
> MS_PIN_DECL(V1, GPIOL6, DASHV1, TXD1); FUNC_GROUP_DECL(TXD1, V1);
>
> #define W1 95
> #define W1_DESC SIG_DESC_SET(SCU84, 23)
> SIG_EXPR_LIST_DECL_SINGLE(DASHW1, DASHW1, VPIRSVD_DESC, W1_DESC);
> -SIG_EXPR_LIST_DECL_SINGLE(RXD1, RXD1, W1_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(RXD1, RXD1, W1_DESC);
> MS_PIN_DECL(W1, GPIOL7, DASHW1, RXD1); FUNC_GROUP_DECL(RXD1, W1);
>
> @@ -766,14 +766,14 @@ FUNC_GROUP_DECL(RXD2, T5); #define V2 104
> #define V2_DESC SIG_DESC_SET(SCU88, 0)
> SIG_EXPR_LIST_DECL_SINGLE(DASHN0, DASHN0, VPIRSVD_DESC, V2_DESC);
> -SIG_EXPR_LIST_DECL_SINGLE(PWM0, PWM0, V2_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(PWM0, PWM0, V2_DESC);
> MS_PIN_DECL(V2, GPION0, DASHN0, PWM0); FUNC_GROUP_DECL(PWM0, V2);
>
> #define W2 105
> #define W2_DESC SIG_DESC_SET(SCU88, 1)
> SIG_EXPR_LIST_DECL_SINGLE(DASHN1, DASHN1, VPIRSVD_DESC, W2_DESC);
> -SIG_EXPR_LIST_DECL_SINGLE(PWM1, PWM1, W2_DESC, COND2);
> +SIG_EXPR_LIST_DECL_SINGLE(PWM1, PWM1, W2_DESC);
> MS_PIN_DECL(W2, GPION1, DASHN1, PWM1); FUNC_GROUP_DECL(PWM1, W2);
>>This looks reasonable to me. I'd like Ryan to chime in though.
>>Ryan, can you confirm the datasheet needs correction here?
>>Tim: You need to send these to a broader audience than the linux-aspeed@ list. Please use ./scripts/get_maintainer.pl to >>determine the appropriate people to send to. This at least needs to go to Linus Walleij, who maintains pinctrl.
>
> --
> 2.11.0
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox