* [PATCH fpga 9/9] fpga: Remove support for non-sg drivers
From: Joshua Clayton @ 2016-11-10 22:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110163315.GB22004@obsidianresearch.com>
On 11/10/2016 08:33 AM, Jason Gunthorpe wrote:
>>> struct fpga_manager_ops {
>>> enum fpga_mgr_states (*state)(struct fpga_manager *mgr);
>>> - int (*write_init)(struct fpga_manager *mgr, u32 flags,
>>> - const char *buf, size_t count);
>>> - int (*write)(struct fpga_manager *mgr, const char *buf, size_t count);
>>> int (*write_init_sg)(struct fpga_manager *mgr, u32 flags,
>>> struct sg_table *sgt);
>>> int (*write_sg)(struct fpga_manager *mgr, struct sg_table *sgt);
>>> @@ -118,6 +113,8 @@ struct fpga_manager {
>>>
>>> int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags,
>>> const char *buf, size_t count);
>>> +int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, u32 flags,
>>> + struct sg_table *sgt);
>>>
>>> int fpga_mgr_firmware_load(struct fpga_manager *mgr, u32 flags,
>>> const char *image_name);
>> I don't have any feeling either way about switching to scatter-gather.
>> (Not zynq or socfpga user)
>> But I do object to renaming the API.
>> write_init() and write() do not imply a particular implementation, nor even that
>> the buffer is coherent.
> Neither the sg or old linear interface imply any particular
> underlying driver implementation.
>
> All that is being changed is how the list of physical pages gets
> passed to the driver. The linear interface requires them to be
> contiguously mapped (eg in a vmap) while the SG interface
> directly passes a list of physical page addresses.
>
> Any alogrithm that works with the old interface can run on the new
> interface, and the new interface can support much better options for
> DMA drivers, while not requiring the higher layers to perform a high
> order allocation (vmap or otherwise) to create the contiguous memory.
>
> The reason the old interface is being deleted here is so the fpga mgr
> API can be expanded to accept a sg list directly. Since we cannot
> convert a general sg list to linear memory the liner option must be
> totally removed.
OK. That sounds reasonable.
>> I am working to merge an fpga manager which uses SPI to load the bitstream
>> (see https://www.spinics.net/lists/arm-kernel/msg539328.html)
>> Any dma in use there would come from the spi driver. write_init_sg, and write_sg
>> don't make any sense in my case.
> No, it still make lots of sense.
>
> SPI has been slowly transforming to use the same sort of SG scheme
> universally, including facing the client. (see
> 6ad45a27cbe343ec8d7888e5edf6335499a4b555)
Thanks for sharing that link.
>
> Some day your driver can just pass the SGs directly to spi and
> everything will be great.
>
> In the mean time it can do sg_miter_next to get mapped buffers.
..so I have a way forward if this series gets merged.
That was my main concern.
And as a dma n00b, learning to use dma engine structures to do
non-dma xfer was not very high on my list.
>> Would it not make sense to keep the top level API the same?
> Fundamentally no.
>
> Jason
Joshua
^ permalink raw reply
* [PATCH v6 5/9] drm/hisilicon/hibmc: Add crtc for DE
From: Sean Paul @ 2016-11-10 22:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-6-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Add crtc funcs and helper funcs for DE.
>
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> ---
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 318 ++++++++++++++++++++++++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
> 3 files changed, 326 insertions(+)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> index 9c1a68c..9b5d0d0 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> @@ -23,6 +23,7 @@
>
> #include "hibmc_drm_drv.h"
> #include "hibmc_drm_regs.h"
> +#include "hibmc_drm_de.h"
> #include "hibmc_drm_power.h"
nit: alphabetize
>
> /* ---------------------------------------------------------------------- */
Remove
> @@ -168,3 +169,320 @@ int hibmc_plane_init(struct hibmc_drm_device *hidev)
> drm_plane_helper_add(plane, &hibmc_plane_helper_funcs);
> return 0;
> }
> +
> +static void hibmc_crtc_enable(struct drm_crtc *crtc)
> +{
> + unsigned int reg;
> + /* power mode 0 is default. */
This comment seems to be in the wrong place
> + struct hibmc_drm_device *hidev = crtc->dev->dev_private;
> +
> + hibmc_set_power_mode(hidev, HIBMC_PW_MODE_CTL_MODE_MODE0);
> +
> + /* Enable display power gate & LOCALMEM power gate*/
> + reg = readl(hidev->mmio + HIBMC_CURRENT_GATE);
> + reg &= ~HIBMC_CURR_GATE_LOCALMEM_MASK;
> + reg &= ~HIBMC_CURR_GATE_DISPLAY_MASK;
> + reg |= HIBMC_CURR_GATE_LOCALMEM(ON);
> + reg |= HIBMC_CURR_GATE_DISPLAY(ON);
> + hibmc_set_current_gate(hidev, reg);
> + drm_crtc_vblank_on(crtc);
> +}
> +
> +static void hibmc_crtc_disable(struct drm_crtc *crtc)
> +{
> + unsigned int reg;
> + struct hibmc_drm_device *hidev = crtc->dev->dev_private;
> +
> + drm_crtc_vblank_off(crtc);
> +
> + hibmc_set_power_mode(hidev, HIBMC_PW_MODE_CTL_MODE_SLEEP);
> +
> + /* Enable display power gate & LOCALMEM power gate*/
> + reg = readl(hidev->mmio + HIBMC_CURRENT_GATE);
> + reg &= ~HIBMC_CURR_GATE_LOCALMEM_MASK;
> + reg &= ~HIBMC_CURR_GATE_DISPLAY_MASK;
> + reg |= HIBMC_CURR_GATE_LOCALMEM(OFF);
> + reg |= HIBMC_CURR_GATE_DISPLAY(OFF);
> + hibmc_set_current_gate(hidev, reg);
> +}
> +
> +static int hibmc_crtc_atomic_check(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + return 0;
> +}
Caller NULL-checks, no need for stub
> +
> +static unsigned int format_pll_reg(void)
> +{
> + unsigned int pllreg = 0;
> + struct panel_pll pll = {0};
> +
> + /* Note that all PLL's have the same format. Here,
> + * we just use Panel PLL parameter to work out the bit
> + * fields in the register.On returning a 32 bit number, the value can
> + * be applied to any PLL in the calling function.
> + */
> + pllreg |= HIBMC_PLL_CTRL_BYPASS(OFF) & HIBMC_PLL_CTRL_BYPASS_MASK;
> + pllreg |= HIBMC_PLL_CTRL_POWER(ON) & HIBMC_PLL_CTRL_POWER_MASK;
> + pllreg |= HIBMC_PLL_CTRL_INPUT(OSC) & HIBMC_PLL_CTRL_INPUT_MASK;
> + pllreg |= HIBMC_PLL_CTRL_POD(pll.POD) & HIBMC_PLL_CTRL_POD_MASK;
> + pllreg |= HIBMC_PLL_CTRL_OD(pll.OD) & HIBMC_PLL_CTRL_OD_MASK;
> + pllreg |= HIBMC_PLL_CTRL_N(pll.N) & HIBMC_PLL_CTRL_N_MASK;
> + pllreg |= HIBMC_PLL_CTRL_M(pll.M) & HIBMC_PLL_CTRL_M_MASK;
> +
> + return pllreg;
> +}
> +
> +static void set_vclock_hisilicon(struct drm_device *dev, unsigned long pll)
> +{
> + unsigned long tmp0, tmp1;
> + struct hibmc_drm_device *hidev = dev->dev_private;
> +
> + /* 1. outer_bypass_n=0 */
> + tmp0 = readl(hidev->mmio + CRT_PLL1_HS);
> + tmp0 &= 0xBFFFFFFF;
> + writel(tmp0, hidev->mmio + CRT_PLL1_HS);
> +
> + /* 2. pll_pd=1?inter_bypass=1 */
> + writel(0x21000000, hidev->mmio + CRT_PLL1_HS);
> +
> + /* 3. config pll */
> + writel(pll, hidev->mmio + CRT_PLL1_HS);
> +
> + /* 4. delay */
> + mdelay(1);
These should be usleep_range() see
https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
> +
> + /* 5. pll_pd =0 */
> + tmp1 = pll & ~0x01000000;
> + writel(tmp1, hidev->mmio + CRT_PLL1_HS);
> +
> + /* 6. delay */
> + mdelay(1);
> +
> + /* 7. inter_bypass=0 */
> + tmp1 &= ~0x20000000;
> + writel(tmp1, hidev->mmio + CRT_PLL1_HS);
> +
> + /* 8. delay */
> + mdelay(1);
> +
> + /* 9. outer_bypass_n=1 */
> + tmp1 |= 0x40000000;
> + writel(tmp1, hidev->mmio + CRT_PLL1_HS);
This function is a whole lot of magic. Any chance you can pull the
values out into #defines?
> +}
> +
> +/* This function takes care the extra registers and bit fields required to
nit: multi-line comments have a leading /* line with the comment
starting on the following line
applies below as well
> + *setup a mode in board.
nit: space between * and comment, ie: * setup a mode in board
applies to the rest of the comment too
> + *Explanation about Display Control register:
> + *FPGA only supports 7 predefined pixel clocks, and clock select is
> + *in bit 4:0 of new register 0x802a8.
> + */
> +static unsigned int display_ctrl_adjust(struct drm_device *dev,
> + struct drm_display_mode *mode,
> + unsigned int ctrl)
> +{
> + unsigned long x, y;
> + unsigned long pll1; /* bit[31:0] of PLL */
> + unsigned long pll2; /* bit[63:32] of PLL */
> + struct hibmc_drm_device *hidev = dev->dev_private;
> +
> + x = mode->hdisplay;
> + y = mode->vdisplay;
> +
> + /* Hisilicon has to set up a new register for PLL control
> + *(CRT_PLL1_HS & CRT_PLL2_HS).
> + */
> + if (x == 800 && y == 600) {
> + pll1 = CRT_PLL1_HS_40MHZ;
> + pll2 = CRT_PLL2_HS_40MHZ;
> + } else if (x == 1024 && y == 768) {
> + pll1 = CRT_PLL1_HS_65MHZ;
> + pll2 = CRT_PLL2_HS_65MHZ;
> + } else if (x == 1152 && y == 864) {
> + pll1 = CRT_PLL1_HS_80MHZ_1152;
> + pll2 = CRT_PLL2_HS_80MHZ;
> + } else if (x == 1280 && y == 768) {
> + pll1 = CRT_PLL1_HS_80MHZ;
> + pll2 = CRT_PLL2_HS_80MHZ;
> + } else if (x == 1280 && y == 720) {
> + pll1 = CRT_PLL1_HS_74MHZ;
> + pll2 = CRT_PLL2_HS_74MHZ;
> + } else if (x == 1280 && y == 960) {
> + pll1 = CRT_PLL1_HS_108MHZ;
> + pll2 = CRT_PLL2_HS_108MHZ;
> + } else if (x == 1280 && y == 1024) {
> + pll1 = CRT_PLL1_HS_108MHZ;
> + pll2 = CRT_PLL2_HS_108MHZ;
> + } else if (x == 1600 && y == 1200) {
> + pll1 = CRT_PLL1_HS_162MHZ;
> + pll2 = CRT_PLL2_HS_162MHZ;
> + } else if (x == 1920 && y == 1080) {
> + pll1 = CRT_PLL1_HS_148MHZ;
> + pll2 = CRT_PLL2_HS_148MHZ;
> + } else if (x == 1920 && y == 1200) {
> + pll1 = CRT_PLL1_HS_193MHZ;
> + pll2 = CRT_PLL2_HS_193MHZ;
> + } else /* default to VGA clock */ {
> + pll1 = CRT_PLL1_HS_25MHZ;
> + pll2 = CRT_PLL2_HS_25MHZ;
> + }
This seems like something that should be checked in atomic_check so
you can be sure the mode is supported.
It would also be nice to pull this out into a separate function (and a
lookup table if you're feeling adventurous)
> +
> + writel(pll2, hidev->mmio + CRT_PLL2_HS);
> + set_vclock_hisilicon(dev, pll1);
> +
> + /* Hisilicon has to set up the top-left and bottom-right
> + * registers as well.
> + * Note that normal chip only use those two register for
> + * auto-centering mode.
> + */
> + writel((HIBMC_CRT_AUTO_CENTERING_TL_TOP(0) &
> + HIBMC_CRT_AUTO_CENTERING_TL_TOP_MSK) |
> + (HIBMC_CRT_AUTO_CENTERING_TL_LEFT(0) &
> + HIBMC_CRT_AUTO_CENTERING_TL_LEFT_MSK),
> + hidev->mmio + HIBMC_CRT_AUTO_CENTERING_TL);
> +
> + writel((HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM(y - 1) &
> + HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
> + (HIBMC_CRT_AUTO_CENTERING_BR_RIGHT(x - 1) &
> + HIBMC_CRT_AUTO_CENTERING_BR_RIGHT_MASK),
> + hidev->mmio + HIBMC_CRT_AUTO_CENTERING_BR);
> +
> + /* Assume common fields in ctrl have been properly set before
> + * calling this function.
> + * This function only sets the extra fields in ctrl.
> + */
> +
> + /* Set bit 25 of display controller: Select CRT or VGA clock */
> + ctrl &= ~HIBMC_CRT_DISP_CTL_CRTSELECT_MASK;
> + ctrl &= ~HIBMC_CRT_DISP_CTL_CLOCK_PHASE_MASK;
> +
> + ctrl |= HIBMC_CRT_DISP_CTL_CRTSELECT(CRTSELECT_CRT);
> +
> + /*ctrl = FIELD_SET(ctrl, HIBMC_CRT_DISP_CTL, CRTSELECT, CRT);*/
What's the deal with this commented code?
> +
> + /* Set bit 14 of display controller */
> + /*ctrl &= FIELD_CLEAR(HIBMC_CRT_DISP_CTL, CLOCK_PHASE);*/
> +
> + /* clock_phase_polarity is 0 */
> + ctrl |= HIBMC_CRT_DISP_CTL_CLOCK_PHASE(PHASE_ACTIVE_HIGH);
> + /*ctrl = FIELD_SET(ctrl, HIBMC_CRT_DISP_CTL,*/
> + /*CLOCK_PHASE, ACTIVE_HIGH);*/
Here too...
> +
> + writel(ctrl, hidev->mmio + HIBMC_CRT_DISP_CTL);
> +
> + return ctrl;
> +}
> +
> +static void hibmc_crtc_mode_set_nofb(struct drm_crtc *crtc)
> +{
> + unsigned int val;
> + struct drm_display_mode *mode = &crtc->state->mode;
> + struct drm_device *dev = crtc->dev;
> + struct hibmc_drm_device *hidev = dev->dev_private;
> +
> + writel(format_pll_reg(), hidev->mmio + HIBMC_CRT_PLL_CTRL);
> + writel((HIBMC_CRT_HORZ_TOTAL_TOTAL(mode->htotal - 1) &
> + HIBMC_CRT_HORZ_TOTAL_TOTAL_MASK) |
> + (HIBMC_CRT_HORZ_TOTAL_DISPLAY_END(mode->hdisplay - 1) &
> + HIBMC_CRT_HORZ_TOTAL_DISPLAY_END_MASK),
You could probably macroize this code to make it more readable
> + hidev->mmio + HIBMC_CRT_HORZ_TOTAL);
> +
> + writel((HIBMC_CRT_HORZ_SYNC_WIDTH(mode->hsync_end - mode->hsync_start)
> + & HIBMC_CRT_HORZ_SYNC_WIDTH_MASK) |
> + (HIBMC_CRT_HORZ_SYNC_START(mode->hsync_start - 1)
> + & HIBMC_CRT_HORZ_SYNC_START_MASK),
> + hidev->mmio + HIBMC_CRT_HORZ_SYNC);
> +
> + writel((HIBMC_CRT_VERT_TOTAL_TOTAL(mode->vtotal - 1) &
> + HIBMC_CRT_VERT_TOTAL_TOTAL_MASK) |
> + (HIBMC_CRT_VERT_TOTAL_DISPLAY_END(mode->vdisplay - 1) &
> + HIBMC_CRT_VERT_TOTAL_DISPLAY_END_MASK),
> + hidev->mmio + HIBMC_CRT_VERT_TOTAL);
> +
> + writel((HIBMC_CRT_VERT_SYNC_HEIGHT(mode->vsync_end - mode->vsync_start)
> + & HIBMC_CRT_VERT_SYNC_HEIGHT_MASK) |
> + (HIBMC_CRT_VERT_SYNC_START(mode->vsync_start - 1) &
> + HIBMC_CRT_VERT_SYNC_START_MASK),
> + hidev->mmio + HIBMC_CRT_VERT_SYNC);
> +
> + val = HIBMC_CRT_DISP_CTL_VSYNC_PHASE(0) &
> + HIBMC_CRT_DISP_CTL_VSYNC_PHASE_MASK;
> + val |= HIBMC_CRT_DISP_CTL_HSYNC_PHASE(0) &
> + HIBMC_CRT_DISP_CTL_HSYNC_PHASE_MASK;
> + val |= HIBMC_CRT_DISP_CTL_TIMING(ENABLE);
> + val |= HIBMC_CRT_DISP_CTL_PLANE(ENABLE);
> +
> + display_ctrl_adjust(dev, mode, val);
> +}
> +
> +static void hibmc_crtc_atomic_begin(struct drm_crtc *crtc,
> + struct drm_crtc_state *old_state)
> +{
> + unsigned int reg;
> + struct drm_device *dev = crtc->dev;
> + struct hibmc_drm_device *hidev = dev->dev_private;
> +
> + hibmc_set_power_mode(hidev, HIBMC_PW_MODE_CTL_MODE_MODE0);
> +
> + /* Enable display power gate & LOCALMEM power gate*/
> + reg = readl(hidev->mmio + HIBMC_CURRENT_GATE);
> + reg &= ~HIBMC_CURR_GATE_DISPLAY_MASK;
> + reg &= ~HIBMC_CURR_GATE_LOCALMEM_MASK;
> + reg |= HIBMC_CURR_GATE_DISPLAY(ON);
> + reg |= HIBMC_CURR_GATE_LOCALMEM(ON);
> + hibmc_set_current_gate(hidev, reg);
> +
> + /* We can add more initialization as needed. */
> +}
> +
> +static void hibmc_crtc_atomic_flush(struct drm_crtc *crtc,
> + struct drm_crtc_state *old_state)
> +
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&crtc->dev->event_lock, flags);
> + if (crtc->state->event)
> + drm_crtc_send_vblank_event(crtc, crtc->state->event);
> + crtc->state->event = NULL;
> +
> + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> +}
> +
> +/* These provide the minimum set of functions required to handle a CRTC */
nit: don't need this comment
> +static const struct drm_crtc_funcs hibmc_crtc_funcs = {
> + .page_flip = drm_atomic_helper_page_flip,
> + .set_config = drm_atomic_helper_set_config,
> + .destroy = drm_crtc_cleanup,
> + .reset = drm_atomic_helper_crtc_reset,
> + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +static const struct drm_crtc_helper_funcs hibmc_crtc_helper_funcs = {
> + .enable = hibmc_crtc_enable,
> + .disable = hibmc_crtc_disable,
> + .mode_set_nofb = hibmc_crtc_mode_set_nofb,
> + .atomic_check = hibmc_crtc_atomic_check,
> + .atomic_begin = hibmc_crtc_atomic_begin,
> + .atomic_flush = hibmc_crtc_atomic_flush,
> +};
> +
> +int hibmc_crtc_init(struct hibmc_drm_device *hidev)
> +{
> + struct drm_device *dev = hidev->dev;
> + struct drm_crtc *crtc = &hidev->crtc;
> + struct drm_plane *plane = &hidev->plane;
> + int ret;
> +
> + ret = drm_crtc_init_with_planes(dev, crtc, plane,
> + NULL, &hibmc_crtc_funcs, NULL);
> + if (ret) {
> + DRM_ERROR("failed to init crtc.\n");
print return code
> + return ret;
> + }
> +
> + drm_mode_crtc_set_gamma_size(crtc, 256);
check return code
> + drm_crtc_helper_add(crtc, &hibmc_crtc_helper_funcs);
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 7d96583..303cd36 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -119,6 +119,12 @@ static int hibmc_kms_init(struct hibmc_drm_device *hidev)
> return ret;
> }
>
> + ret = hibmc_crtc_init(hidev);
> + if (ret) {
> + DRM_ERROR("failed to init crtc.\n");
> + return ret;
> + }
Typically the plane is initialized internally in the crtc driver. I
think this is a good design pattern, and you should probably use it.
So how about squashing this down with the plane patch and keeping the
plane inside hibmc_drm_de.c?
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 49e39d2..5731ec2 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -46,6 +46,7 @@ struct hibmc_drm_device {
> /* drm */
> struct drm_device *dev;
> struct drm_plane plane;
I don't think you should be keeping track of plane here. plane is only
used in the crtc init, which should be addressed by the previous
comment.
> + struct drm_crtc crtc;
crtc is only used in the irq handler, so you could remove this here
and just call drm_handle_vblank(dev, 0) in the handler.
> bool mode_config_initialized;
>
> /* ttm */
> @@ -85,6 +86,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
> #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
>
> int hibmc_plane_init(struct hibmc_drm_device *hidev);
> +int hibmc_crtc_init(struct hibmc_drm_device *hidev);
> int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
> void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 6/9] drm/hisilicon/hibmc: Add encoder for VDAC
From: Sean Paul @ 2016-11-10 22:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-7-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Add encoder funcs and helpers for VDAC.
>
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> ---
> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 ++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 89 ++++++++++++++++++++++++
> 4 files changed, 98 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index 72e107e..e04f114 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -1,5 +1,5 @@
> ccflags-y := -Iinclude/drm
> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_fbdev.o hibmc_drm_power.o hibmc_ttm.o
>
> obj-$(CONFIG_DRM_HISI_HIBMC) +=hibmc-drm.o
> #obj-y += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 303cd36..ba191e1 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -125,6 +125,12 @@ static int hibmc_kms_init(struct hibmc_drm_device *hidev)
> return ret;
> }
>
> + ret = hibmc_encoder_init(hidev);
> + if (ret) {
> + DRM_ERROR("failed to init encoder\n");
> + return ret;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 5731ec2..401cea4 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -47,6 +47,7 @@ struct hibmc_drm_device {
> struct drm_device *dev;
> struct drm_plane plane;
> struct drm_crtc crtc;
> + struct drm_encoder encoder;
Same comment here, you don't need to keep track of this
> bool mode_config_initialized;
>
> /* ttm */
> @@ -87,6 +88,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
>
> int hibmc_plane_init(struct hibmc_drm_device *hidev);
> int hibmc_crtc_init(struct hibmc_drm_device *hidev);
> +int hibmc_encoder_init(struct hibmc_drm_device *hidev);
> int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
> void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> new file mode 100644
> index 0000000..953f659
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -0,0 +1,89 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Rongrong Zou <zourongrong@huawei.com>
> + * Rongrong Zou <zourongrong@gmail.com>
> + * Jianhua Li <lijianhua@huawei.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +#include "hibmc_drm_drv.h"
> +#include "hibmc_drm_regs.h"
> +
> +static int defx = 800;
> +static int defy = 600;
> +
> +module_param(defx, int, 0444);
> +module_param(defy, int, 0444);
> +MODULE_PARM_DESC(defx, "default x resolution");
> +MODULE_PARM_DESC(defy, "default y resolution");
Not used, and I'm not sure these are a good idea
> +
> +static void hibmc_encoder_disable(struct drm_encoder *encoder)
> +{
> +}
> +
> +static void hibmc_encoder_enable(struct drm_encoder *encoder)
> +{
> +}
Null-checked, no need to stub
> +
> +static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
> + struct drm_display_mode *mode,
> + struct drm_display_mode *adj_mode)
> +{
> + u32 reg;
> + struct drm_device *dev = encoder->dev;
> + struct hibmc_drm_device *hidev = dev->dev_private;
> +
> + /* just open DISPLAY_CONTROL_HISILE register bit 3:0*/
> + reg = readl(hidev->mmio + DISPLAY_CONTROL_HISILE);
> + reg |= 0xf;
Can you just pull this into a #define instead of explaining in the comment?
> + writel(reg, hidev->mmio + DISPLAY_CONTROL_HISILE);
> +}
> +
> +static int hibmc_encoder_atomic_check(struct drm_encoder *encoder,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + return 0;
> +}
null-checked, remove stub
> +
> +static const struct drm_encoder_helper_funcs hibmc_encoder_helper_funcs = {
> + .mode_set = hibmc_encoder_mode_set,
> + .disable = hibmc_encoder_disable,
> + .enable = hibmc_encoder_enable,
> + .atomic_check = hibmc_encoder_atomic_check,
> +};
> +
> +static const struct drm_encoder_funcs hibmc_encoder_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> +int hibmc_encoder_init(struct hibmc_drm_device *hidev)
> +{
> + struct drm_device *dev = hidev->dev;
> + struct drm_encoder *encoder = &hidev->encoder;
> + int ret;
> +
> + encoder->possible_crtcs = 0x1;
> + ret = drm_encoder_init(dev, encoder, &hibmc_encoder_encoder_funcs,
> + DRM_MODE_ENCODER_DAC, NULL);
> + if (ret) {
> + DRM_ERROR("failed to init encoder\n");
print ret
> + return ret;
> + }
> +
> + drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
> + return 0;
> +}
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/2] mmc: sdhci-iproc: Add brcm,sdhci-iproc compat string in bindings document
From: Ulf Hansson @ 2016-11-10 22:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478018277-10097-2-git-send-email-scott.branden@broadcom.com>
On 1 November 2016 at 17:37, Scott Branden <scott.branden@broadcom.com> wrote:
> Adds brcm,sdhci-iproc compat string to DT bindings document for
> the iProc SDHCI driver.
>
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
> Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> index be56d2b..954561d 100644
> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> @@ -7,6 +7,15 @@ Required properties:
> - compatible : Should be one of the following
> "brcm,bcm2835-sdhci"
> "brcm,sdhci-iproc-cygnus"
> + "brcm,sdhci-iproc"
> +
> +Use brcm2835-sdhci for Rasperry PI.
> +
> +Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
> +restricted to 32bit host accesses to SDHCI registers.
> +
> +Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
> +8, 16, 32-bit host access to SDHCI register.
>
> - clocks : The clock feeding the SDHCI controller.
>
> --
> 2.5.0
>
^ permalink raw reply
* [PATCH v2 2/2] mmc: sdhci-iproc: support standard byte register accesses
From: Ulf Hansson @ 2016-11-10 22:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478018277-10097-3-git-send-email-scott.branden@broadcom.com>
On 1 November 2016 at 17:37, Scott Branden <scott.branden@broadcom.com> wrote:
> Add bytewise register accesses support for newer versions of IPROC
> SDHCI controllers.
> Previous sdhci-iproc versions of SDIO controllers
> (such as Raspberry Pi and Cygnus) only allowed for 32-bit register
> accesses.
>
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-iproc.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 7262466..d7046d6 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -143,6 +143,14 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, u8 val, int reg)
> }
>
> static const struct sdhci_ops sdhci_iproc_ops = {
> + .set_clock = sdhci_set_clock,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> +};
> +
> +static const struct sdhci_ops sdhci_iproc_32only_ops = {
> .read_l = sdhci_iproc_readl,
> .read_w = sdhci_iproc_readw,
> .read_b = sdhci_iproc_readb,
> @@ -156,6 +164,28 @@ static const struct sdhci_ops sdhci_iproc_ops = {
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> };
>
> +static const struct sdhci_pltfm_data sdhci_iproc_cygnus_pltfm_data = {
> + .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
> + .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN,
> + .ops = &sdhci_iproc_32only_ops,
> +};
> +
> +static const struct sdhci_iproc_data iproc_cygnus_data = {
> + .pdata = &sdhci_iproc_cygnus_pltfm_data,
> + .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> + & SDHCI_MAX_BLOCK_MASK) |
> + SDHCI_CAN_VDD_330 |
> + SDHCI_CAN_VDD_180 |
> + SDHCI_CAN_DO_SUSPEND |
> + SDHCI_CAN_DO_HISPD |
> + SDHCI_CAN_DO_ADMA2 |
> + SDHCI_CAN_DO_SDMA,
> + .caps1 = SDHCI_DRIVER_TYPE_C |
> + SDHCI_DRIVER_TYPE_D |
> + SDHCI_SUPPORT_DDR50,
> + .mmc_caps = MMC_CAP_1_8V_DDR,
> +};
> +
> static const struct sdhci_pltfm_data sdhci_iproc_pltfm_data = {
> .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
> .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN,
> @@ -182,7 +212,7 @@ static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
> .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> SDHCI_QUIRK_MISSING_CAPS,
> - .ops = &sdhci_iproc_ops,
> + .ops = &sdhci_iproc_32only_ops,
> };
>
> static const struct sdhci_iproc_data bcm2835_data = {
> @@ -194,7 +224,8 @@ static const struct sdhci_iproc_data bcm2835_data = {
>
> static const struct of_device_id sdhci_iproc_of_match[] = {
> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data },
> - { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_data },
> + { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_cygnus_data},
> + { .compatible = "brcm,sdhci-iproc", .data = &iproc_data },
> { }
> };
> MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match);
> --
> 2.5.0
>
^ permalink raw reply
* [PATCH] staging: vc04_services: add vchiq_pagelist_info structure
From: Greg KH @ 2016-11-10 22:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478533287.17065.1.camel@crowfest.net>
On Mon, Nov 07, 2016 at 07:41:27AM -0800, Michael Zoran wrote:
> On Mon, 2016-11-07 at 11:03 +0100, Greg KH wrote:
> > On Mon, Oct 31, 2016 at 01:10:35AM -0700, Michael Zoran wrote:
> > > The current dma_map_sg based implementation for bulk messages
> > > computes many offsets into a single allocation multiple times in
> > > both the create and free code paths.??This is inefficient,
> > > error prone and in fact still has a few lingering issues
> > > with arm64.
> > >
> > > This change replaces a small portion of that inplementation with
> > > new code that uses a new struct vchiq_pagelist_info to store the
> > > needed information rather then complex offset calculations.
> > >
> > > This improved implementation should be more efficient and easier
> > > to understand and maintain.
> > >
> > > Tests Run(Both Pass):
> > > vchiq_test -p 1
> > > vchiq_test -f 10
> > >
> > > Signed-off-by: Michael Zoran <mzoran@crowfest.net>
> > > ---
> > > ?.../interface/vchiq_arm/vchiq_2835_arm.c???????????| 223
> > > +++++++++++----------
> > > ?1 file changed, 113 insertions(+), 110 deletions(-)
> >
> > This doesn't apply to the tree anymore because of your previous patch
> > :(
> >
> > Can you refresh it and resend?
> >
> > thanks,
> >
> > greg k-h
>
> OK, I resubmitted it.
>
> Once this patch gets applied 64-bit should be in decent shape. I'm not
> seeing any warnings or errors anymore and functional tests from a 64-
> bit OS look good.
>
> The only remaining issue that I know of is that it needs a 32-bit
> compatibility layer for the ioctls when running a 32-bit OS(Raspbian)
> on top of a 64-bit kernel.
Ok, let's turn off BROKEN for the module, and enable CONFIG_TEST and see
if the 0-day bot barfs all over it or not :)
thanks,
greg k-h
^ permalink raw reply
* [Linaro-acpi] ACPI namespace details for ARM64
From: Al Stone @ 2016-11-10 23:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109220506.GN14322@bhelgaas-glaptop.roam.corp.google.com>
On 11/09/2016 03:05 PM, Bjorn Helgaas wrote:
> Hi all,
>
> We've been working through the details of getting ACPI to work on
> arm64, and there have been lots of questions about what this means for
> PCI. I've outlined this for several people individually, but I'm
> going to send this separately, apart from a specific patch series, to
> make sure we're all on the same page. Please correct my errors and
> misunderstandings.
>
> Bjorn
>
>[snip....]
A big +1 to all of this. This also looks like something that should
be added to either PCI, ACPI or arm64 documentation (or even all three).
What do you think?
Thank you for putting this together, Bjorn.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------
^ permalink raw reply
* [clk:clk-qcom-8994 2/2] include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
From: kbuild test robot @ 2016-11-10 23:20 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-qcom-8994
head: cc800227108710c8f02255e61659b956b041eec3
commit: cc800227108710c8f02255e61659b956b041eec3 [2/2] clk: qcom: Add support for msm8994 global clock controller
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout cc800227108710c8f02255e61659b956b041eec3
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from drivers/clk/qcom/gcc-msm8994.c:20:0:
>> include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
extern const typeof(name) __mod_##type##__##name##_device_table \
^
>> drivers/clk/qcom/gcc-msm8994.c:2265:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
^~~~~~~~~~~~~~~~~~~
vim +213 include/linux/module.h
^1da177e Linus Torvalds 2005-04-16 207 /* What your module does. */
^1da177e Linus Torvalds 2005-04-16 208 #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
^1da177e Linus Torvalds 2005-04-16 209
cff26a51 Rusty Russell 2014-02-03 210 #ifdef MODULE
cff26a51 Rusty Russell 2014-02-03 211 /* Creates an alias so file2alias.c can find device table. */
^1da177e Linus Torvalds 2005-04-16 212 #define MODULE_DEVICE_TABLE(type, name) \
6301939d Andrey Ryabinin 2015-02-13 @213 extern const typeof(name) __mod_##type##__##name##_device_table \
cff26a51 Rusty Russell 2014-02-03 214 __attribute__ ((unused, alias(__stringify(name))))
cff26a51 Rusty Russell 2014-02-03 215 #else /* !MODULE */
cff26a51 Rusty Russell 2014-02-03 216 #define MODULE_DEVICE_TABLE(type, name)
:::::: The code at line 213 was first introduced by commit
:::::: 6301939d97d079f0d3dbe71e750f4daf5d39fc33 module: fix types of device tables aliases
:::::: TO: Andrey Ryabinin <a.ryabinin@samsung.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 56819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161111/9bf381ab/attachment-0001.gz>
^ permalink raw reply
* [PATCH v7] soc: qcom: add l2 cache perf events driver
From: Leeder, Neil @ 2016-11-10 23:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109175413.GE17020@leverpostej>
Hi Mark,
Thanks for the review. I'll handle all the syntactic comments, so I
won't reply to them all individually here.
For the aggregation, I'll reply separately to Will's post to
keep all those comments together.
On 11/9/2016 12:54 PM, Mark Rutland wrote:
>> +
>> +/*
>> + * The cache is made up of one or more clusters, each cluster has its own PMU.
>> + * Each cluster is associated with one or more CPUs.
>> + * This structure represents one of the hardware PMUs.
>> + *
>> + * Events can be envisioned as a 2-dimensional array. Each column represents
>> + * a group of events. There are 8 groups. Only one entry from each
>> + * group can be in use at a time. When an event is assigned a counter
>> + * by *_event_add(), the counter index is assigned to group_to_counter[group].
>
> If I've followed correctly, this means each group has a dedicated
> counter?
>
> I take it groups are not symmetric (i.e. each column has different
> events)? Or does each column contain the same events?
>
> Is there any overlap?
Each group will have at most one counter, but it's not dedicated.
There's no requirement that an implementation have as many counters as
there are groups, so an event can be assigned to any available counter.
Every entry in the 2-dimensional array is unique, so no duplicates. Once
you have used an event, you cannot use any other event from its column.
As an aside, hardware designers put in some effort to ensure that events
which may need to be collected at the same time are located in different
columns.
>> +static int l2_cache__event_init(struct perf_event *event)
>> +{
[...]
>> + /* Don't allow groups with mixed PMUs, except for s/w events */
>> + if (event->group_leader->pmu != event->pmu &&
>> + !is_software_event(event->group_leader)) {
>> + dev_warn(&l2cache_pmu->pdev->dev,
>> + "Can't create mixed PMU group\n");
>> + return -EINVAL;
>> + }
>> +
>> + list_for_each_entry(sibling, &event->group_leader->sibling_list,
>> + group_entry)
>> + if (sibling->pmu != event->pmu &&
>> + !is_software_event(sibling)) {
>> + dev_warn(&l2cache_pmu->pdev->dev,
>> + "Can't create mixed PMU group\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Ensure all events in a group are on the same cpu */
>> + cluster = get_hml2_pmu(event->cpu);
>> + if ((event->group_leader != event) &&
>> + (cluster->on_cpu != event->group_leader->cpu)) {
>> + dev_warn(&l2cache_pmu->pdev->dev,
>> + "Can't create group on CPUs %d and %d",
>> + event->cpu, event->group_leader->cpu);
>> + return -EINVAL;
>> + }
>
> It's probably worth also checking that the events are co-schedulable
> (e.g. they don't conflict column-wise).
That's what filter_match() is doing - stopping column-conflicting events
from even getting to init(). In init() we don't have a record of which
other events are being co-scheduled. We could keep a list of groups used
by other events to compare against, but because there's no matching
term() function there's no obvious way of removing them from the list.
In my very first patchset I had the check inside event_add() because
entries could be removed in event_del(). That was where you suggested
that I use filter_match().
>
>> + if (acpi_bus_get_device(ACPI_HANDLE(dev), &device))
>> + return -ENODEV;
>> +
>> + if (kstrtol(device->pnp.unique_id, 10, &fw_cluster_id) < 0) {
>> + dev_err(&pdev->dev, "unable to read ACPI uid\n");
>> + return -ENODEV;
>> + }
>
>> + cluster->l2cache_pmu = l2cache_pmu;
>> + for_each_present_cpu(cpu) {
>> + if (topology_physical_package_id(cpu) == fw_cluster_id) {
>> + cpumask_set_cpu(cpu, &cluster->cluster_cpus);
>> + per_cpu(pmu_cluster, cpu) = cluster;
>> + }
>> + }
>
> I'm still uneasy about this.
>
> The topology_* API doesn't have a well-defined mapping to the MPIDR.Aff*
> levels, which itself also don't have a well-defined mapping to your
> hardware's clusters (and therefore fw_cluster_id).
>
> Thus, I'm rather worried that this is going to get broken easily, either
> by changes in the kernel, or in future HW revisions where the mapping of
> clusters to MPIDR.Aff* fields changes.
I'm not sure how else to get a mapping of CPU to cluster which doesn't
eventually end with MPIDR.
This is the definition of topology_physical_package_id() from
asm/topology.h:
#define topology_physical_package_id(cpu)
(cpu_topology[cpu].cluster_id)
It seems to be a pretty solid connection between cpu and cluster. I
don't think this is an abuse of this function. Unless there is some
other way of getting this mapping I'd suggest using this, and if some
future chip should change MPIDR usage it can be addressed it then.
>
> Thanks,
> Mark.
>
Thanks,
Neil
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [clk:clk-qcom-8994 2/2] include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
From: Stephen Boyd @ 2016-11-10 23:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201611110732.1ceuZvsr%fengguang.wu@intel.com>
On 11/11, kbuild test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-qcom-8994
> head: cc800227108710c8f02255e61659b956b041eec3
> commit: cc800227108710c8f02255e61659b956b041eec3 [2/2] clk: qcom: Add support for msm8994 global clock controller
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> git checkout cc800227108710c8f02255e61659b956b041eec3
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from drivers/clk/qcom/gcc-msm8994.c:20:0:
> >> include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
> extern const typeof(name) __mod_##type##__##name##_device_table \
> ^
> >> drivers/clk/qcom/gcc-msm8994.c:2265:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
> MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
Urgh that's sad. Too bad MODULE_DEVICE_TABLE doesn't have
something in it in the !MODULE case to cause compilation problems
like this to come out. I'll go fix this up.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH] arm: kprobe: replace patch_lock to raw lock
From: Yang Shi @ 2016-11-11 0:17 UTC (permalink / raw)
To: linux-arm-kernel
When running kprobe on -rt kernel, the below bug is caught:
BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:931
in_atomic(): 1, irqs_disabled(): 128, pid: 14, name: migration/0
INFO: lockdep is turned off.
irq event stamp: 238
hardirqs last enabled at (237): [<80b5aecc>] _raw_spin_unlock_irqrestore+0x88/0x90
hardirqs last disabled at (238): [<80b56d88>] __schedule+0xec/0x94c
softirqs last enabled at (0): [<80225584>] copy_process.part.5+0x30c/0x1994
softirqs last disabled at (0): [< (null)>] (null)
Preemption disabled at:[<802f2b98>] cpu_stopper_thread+0xc0/0x140
CPU: 0 PID: 14 Comm: migration/0 Tainted: G O 4.8.3-rt2 #1
Hardware name: Freescale LS1021A
[<80212e7c>] (unwind_backtrace) from [<8020cd2c>] (show_stack+0x20/0x24)
[<8020cd2c>] (show_stack) from [<80689e14>] (dump_stack+0xa0/0xcc)
[<80689e14>] (dump_stack) from [<8025a43c>] (___might_sleep+0x1b8/0x2a4)
[<8025a43c>] (___might_sleep) from [<80b5b324>] (rt_spin_lock+0x34/0x74)
[<80b5b324>] (rt_spin_lock) from [<80b5c31c>] (__patch_text_real+0x70/0xe8)
[<80b5c31c>] (__patch_text_real) from [<80b5c3ac>] (patch_text_stop_machine+0x18/0x20)
[<80b5c3ac>] (patch_text_stop_machine) from [<802f2920>] (multi_cpu_stop+0xfc/0x134)
[<802f2920>] (multi_cpu_stop) from [<802f2ba0>] (cpu_stopper_thread+0xc8/0x140)
[<802f2ba0>] (cpu_stopper_thread) from [<802563a4>] (smpboot_thread_fn+0x1a4/0x354)
[<802563a4>] (smpboot_thread_fn) from [<80251d38>] (kthread+0x104/0x11c)
[<80251d38>] (kthread) from [<80207f70>] (ret_from_fork+0x14/0x24)
Since patch_text_stop_machine() is called in stop_machine() which disables IRQ,
sleepable lock should be not used in this atomic context, so replace patch_lock
to raw lock.
Signed-off-by: Yang Shi <yang.shi@linaro.org>
---
arch/arm/kernel/patch.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 69bda1a..1f665ac 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -15,7 +15,7 @@ struct patch {
unsigned int insn;
};
-static DEFINE_SPINLOCK(patch_lock);
+static DEFINE_RAW_SPINLOCK(patch_lock);
static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
__acquires(&patch_lock)
@@ -32,7 +32,7 @@ static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
return addr;
if (flags)
- spin_lock_irqsave(&patch_lock, *flags);
+ raw_spin_lock_irqsave(&patch_lock, *flags);
else
__acquire(&patch_lock);
@@ -47,7 +47,7 @@ static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
clear_fixmap(fixmap);
if (flags)
- spin_unlock_irqrestore(&patch_lock, *flags);
+ raw_spin_unlock_irqrestore(&patch_lock, *flags);
else
__release(&patch_lock);
}
--
2.0.2
^ permalink raw reply related
* [PATCH fpga 3/9] fpga zynq: Fix incorrect ISR state on bootup
From: Moritz Fischer @ 2016-11-11 0:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478732303-13718-4-git-send-email-jgunthorpe@obsidianresearch.com>
Hi Jason,
On Wed, Nov 9, 2016 at 2:58 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> It is best practice to clear and mask all interrupts before
> associating the IRQ, and this should be done after the clock
> is enabled.
>
> This corrects a bad result from zynq_fpga_ops_state on bootup
> where left over latched values in INT_STS_OFFSET caused it to
> report an unconfigured FPGA as configured.
>
> After this change the boot up operating state for an unconfigured
> FPGA reports 'unknown'.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
> drivers/fpga/zynq-fpga.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index 86f4377e2b52..40cf0feaca7c 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -460,13 +460,6 @@ static int zynq_fpga_probe(struct platform_device *pdev)
> return priv->irq;
> }
>
> - err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0,
> - dev_name(dev), priv);
> - if (err) {
> - dev_err(dev, "unable to request IRQ\n");
> - return err;
> - }
> -
> priv->clk = devm_clk_get(dev, "ref_clk");
> if (IS_ERR(priv->clk)) {
> dev_err(dev, "input clock not found\n");
> @@ -482,6 +475,16 @@ static int zynq_fpga_probe(struct platform_device *pdev)
> /* unlock the device */
> zynq_fpga_write(priv, UNLOCK_OFFSET, UNLOCK_MASK);
>
> + zynq_fpga_write(priv, INT_MASK_OFFSET, 0xFFFFFFFF);
Named constant please.
> + zynq_fpga_write(priv, INT_STS_OFFSET, IXR_ALL_MASK);
> + err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0, dev_name(dev),
> + priv);
> + if (err) {
> + dev_err(dev, "unable to request IRQ\n");
> + clk_unprepare(priv->clk);
Your enable count is off in that case. This should be a clk_disable_unprepare()
call.
<snip>
clk_prepare_enable()
if(err) {
clk_unprepare();
return err; /* enable count = 1? <- huh? */
}
clk_disable() enable count = 0
clk_unprepare() prepare count = 0
</snip>
> + return err;
> + }
> +
> clk_disable(priv->clk);
>
> err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager",
> --
> 2.1.4
>
Thanks,
Moritz
^ permalink raw reply
* [PATCH fpga 3/9] fpga zynq: Fix incorrect ISR state on bootup
From: Jason Gunthorpe @ 2016-11-11 0:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAtXAHeaaxf4L7aQ455Yrpm1fgqr=L-LxMd7yDQOKjLsBvp38w@mail.gmail.com>
On Thu, Nov 10, 2016 at 04:44:39PM -0800, Moritz Fischer wrote:
> > + zynq_fpga_write(priv, INT_MASK_OFFSET, 0xFFFFFFFF);
>
> Named constant please.
This line gets fixed in the next patch that, lets just leave it, less
churn..
> > + zynq_fpga_write(priv, INT_STS_OFFSET, IXR_ALL_MASK);
> > + err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0, dev_name(dev),
> > + priv);
> > + if (err) {
> > + dev_err(dev, "unable to request IRQ\n");
> > + clk_unprepare(priv->clk);
>
> Your enable count is off in that case. This should be a clk_disable_unprepare()
> call.
Yep.
FWIW, it feels wrong to leave the ISR enabled but the clk
disabled..
Jason
^ permalink raw reply
* [PATCH v9 2/8] power: add power sequence library
From: Rafael J. Wysocki @ 2016-11-11 1:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478573472-29516-3-git-send-email-peter.chen@nxp.com>
On Tue, Nov 8, 2016 at 3:51 AM, Peter Chen <peter.chen@nxp.com> wrote:
> We have an well-known problem that the device needs to do some power
> sequence before it can be recognized by related host, the typical
> example like hard-wired mmc devices and usb devices.
>
> This power sequence is hard to be described at device tree and handled by
> related host driver, so we have created a common power sequence
> library to cover this requirement. The core code has supplied
> some common helpers for host driver, and individual power sequence
> libraries handle kinds of power sequence for devices. The pwrseq
> librares always need to allocate extra instance for compatible
> string match.
>
> pwrseq_generic is intended for general purpose of power sequence, which
> handles gpios and clocks currently, and can cover other controls in
> future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence is needed, else call of_pwrseq_on_list
> /of_pwrseq_off_list instead (eg, USB hub driver).
>
> For new power sequence library, it can add its compatible string
> to pwrseq_of_match_table, then the pwrseq core will match it with
> DT's, and choose this library at runtime.
In the first place, please document this stuff better than you have so
far. To a minimum, add kerneldoc comments to all new non-trivial new
functions to document what they are for and how they are expected to
be used (especially the ones exported to drivers).
Also, is there any guidance available for people who may want to use it?
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> Tested-by Joshua Clayton <stillcompiling@gmail.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> MAINTAINERS | 9 ++
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/pwrseq/Kconfig | 19 ++++
> drivers/power/pwrseq/Makefile | 2 +
> drivers/power/pwrseq/core.c | 191 ++++++++++++++++++++++++++++++++++
> drivers/power/pwrseq/pwrseq_generic.c | 183 ++++++++++++++++++++++++++++++++
> include/linux/power/pwrseq.h | 72 +++++++++++++
> 8 files changed, 478 insertions(+)
> create mode 100644 drivers/power/pwrseq/Kconfig
> create mode 100644 drivers/power/pwrseq/Makefile
> create mode 100644 drivers/power/pwrseq/core.c
> create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
> create mode 100644 include/linux/power/pwrseq.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7..5dab975 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9612,6 +9612,15 @@ F: include/linux/pm_*
> F: include/linux/powercap.h
> F: drivers/powercap/
>
> +POWER SEQUENCE LIBRARY
> +M: Peter Chen <Peter.Chen@nxp.com>
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
> +L: linux-pm at vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/power/pwrseq/
> +F: drivers/power/pwrseq/
> +F: include/linux/power/pwrseq.h/
> +
> POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
> M: Sebastian Reichel <sre@kernel.org>
> L: linux-pm at vger.kernel.org
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 63454b5..c1bb046 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -1,3 +1,4 @@
> source "drivers/power/avs/Kconfig"
> source "drivers/power/reset/Kconfig"
> source "drivers/power/supply/Kconfig"
> +source "drivers/power/pwrseq/Kconfig"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index ff35c71..7db8035 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_POWER_AVS) += avs/
> obj-$(CONFIG_POWER_RESET) += reset/
> obj-$(CONFIG_POWER_SUPPLY) += supply/
> +obj-$(CONFIG_POWER_SEQUENCE) += pwrseq/
> diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
> new file mode 100644
> index 0000000..3859a67
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,19 @@
> +#
> +# Power Sequence library
> +#
> +
> +config POWER_SEQUENCE
> + bool
> +
> +menu "Power Sequence Support"
> +
> +config PWRSEQ_GENERIC
> + bool "Generic power sequence control"
> + depends on OF
> + select POWER_SEQUENCE
> + help
> + It is used for drivers which needs to do power sequence
> + (eg, turn on clock, toggle reset gpio) before the related
> + devices can be found by hardware. This generic one can be
> + used for common power sequence control.
I wouldn't set it up this way.
There are two problems here.
First, say a distro is going to ship a multiplatform generic kernel.
How they are going to figure out whether or not to set the new symbol
in that kernel?
Second, how users are supposed to know whether or not they will need
it even if they build the kernel by themselves?
It would be better IMO to set things up to select the new symbol from
places making use of the code depending on it.
Thanks,
Rafael
^ permalink raw reply
* [PATCH] drm/rockchip: return ERR_PTR instead of NULL
From: Mark yao @ 2016-11-11 1:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478812256-26189-1-git-send-email-Julia.Lawall@lip6.fr>
On 2016?11?11? 05:10, Julia Lawall wrote:
> rockchip_drm_framebuffer_init is only used in one case, in
> rockchip_drm_fbdev.c, where its return value is tested using IS_ERR. To
> enable propagating the reason for the error, change the definition so that
> it returns an ERR_PTR value.
>
> Problem found with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Thanks for the fix.
Applied to my drm-next.
>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 0f6eda0..01e11bf 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -213,7 +213,7 @@ struct drm_framebuffer *
>
> rockchip_fb = rockchip_fb_alloc(dev, mode_cmd, &obj, 1);
> if (IS_ERR(rockchip_fb))
> - return NULL;
> + return ERR_CAST(rockchip_fb);
>
> return &rockchip_fb->fb;
> }
>
>
>
>
--
?ark Yao
^ permalink raw reply
* [PATCH v6 7/9] drm/hisilicon/hibmc: Add connector for VDAC
From: Sean Paul @ 2016-11-11 1:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-8-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Add connector funcs and helper funcs for VDAC.
>
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> ---
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 8 +++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 76 ++++++++++++++++++++++++
> 3 files changed, 86 insertions(+)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index ba191e1..4253603 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -131,6 +131,14 @@ static int hibmc_kms_init(struct hibmc_drm_device *hidev)
> return ret;
> }
>
> + ret = hibmc_connector_init(hidev);
> + if (ret) {
> + DRM_ERROR("failed to init connector\n");
> + return ret;
> + }
> +
> + drm_mode_connector_attach_encoder(&hidev->connector,
> + &hidev->encoder);
The connector should be initialized in the vdac driver with the
encoder, not in the drv file (same as plane/crtc)
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 401cea4..450247d 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -48,6 +48,7 @@ struct hibmc_drm_device {
> struct drm_plane plane;
> struct drm_crtc crtc;
> struct drm_encoder encoder;
> + struct drm_connector connector;
No need to keep track here
> bool mode_config_initialized;
>
> /* ttm */
> @@ -89,6 +90,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
> int hibmc_plane_init(struct hibmc_drm_device *hidev);
> int hibmc_crtc_init(struct hibmc_drm_device *hidev);
> int hibmc_encoder_init(struct hibmc_drm_device *hidev);
> +int hibmc_connector_init(struct hibmc_drm_device *hidev);
> int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
> void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> index 953f659..ebefcd1 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -87,3 +87,79 @@ int hibmc_encoder_init(struct hibmc_drm_device *hidev)
> drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
> return 0;
> }
> +
> +static int hibmc_connector_get_modes(struct drm_connector *connector)
> +{
> + int count;
> +
> + count = drm_add_modes_noedid(connector, 800, 600);
> + drm_set_preferred_mode(connector, defx, defy);
So you have defx/defy as module parameters, but then hardcode the
800x600 mode. If defx/defy is anything other than 800/600, this won't
work. I think you should just remove the defx/defy module params and
rely on userspace adding modes as appropriate.
> + return count;
> +}
> +
> +static int hibmc_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct hibmc_drm_device *hiprivate =
> + container_of(connector, struct hibmc_drm_device, connector);
> + unsigned long size = mode->hdisplay * mode->vdisplay * 4;
Why * 4 here and why * 2 below? You support formats less than 32 bpp,
so the * 4 isn't necessarily correct for all formats. Is the * 2 to
account for front & back buffer?
> +
> + if (size * 2 > hiprivate->fb_size)
> + return MODE_BAD;
> +
> + return MODE_OK;
> +}
> +
> +static struct drm_encoder *
> +hibmc_connector_best_encoder(struct drm_connector *connector)
> +{
> + int enc_id = connector->encoder_ids[0];
> +
> + /* pick the encoder ids */
> + if (enc_id)
> + return drm_encoder_find(connector->dev, enc_id);
Can't you just do return drm_encoder_find(connector->dev,
connector->encoder_ids[0]); ?
ie: won't drm_encoder_find do the right thing if you pass in id == 0?
> +
> + return NULL;
> +}
> +
> +static enum drm_connector_status hibmc_connector_detect(struct drm_connector
> + *connector, bool force)
> +{
> + return connector_status_connected;
Perhaps this should be connector_status_unknown, since you don't
necessarily know it's connected.
> +}
> +
> +static const struct drm_connector_helper_funcs
> + hibmc_connector_connector_helper_funcs = {
> + .get_modes = hibmc_connector_get_modes,
> + .mode_valid = hibmc_connector_mode_valid,
> + .best_encoder = hibmc_connector_best_encoder,
> +};
> +
> +static const struct drm_connector_funcs hibmc_connector_connector_funcs = {
> + .dpms = drm_atomic_helper_connector_dpms,
> + .detect = hibmc_connector_detect,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .reset = drm_atomic_helper_connector_reset,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +int hibmc_connector_init(struct hibmc_drm_device *hidev)
> +{
> + struct drm_device *dev = hidev->dev;
> + struct drm_connector *connector = &hidev->connector;
> + int ret;
> +
> + ret = drm_connector_init(dev, connector,
> + &hibmc_connector_connector_funcs,
> + DRM_MODE_CONNECTOR_VGA);
> + if (ret) {
> + DRM_ERROR("failed to init connector\n");
> + return ret;
> + }
> + drm_connector_helper_add(connector,
> + &hibmc_connector_connector_helper_funcs);
> +
> + return 0;
> +}
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 8/9] drm/hisilicon/hibmc: Add vblank interruput
From: Sean Paul @ 2016-11-11 1:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-9-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Add vblank interrupt.
>
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> ---
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 56 ++++++++++++++++++++++++-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 1 +
> 2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 4253603..b668e3e 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -40,16 +40,46 @@
>
> static int hibmc_enable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> + struct hibmc_drm_device *hidev =
> + (struct hibmc_drm_device *)dev->dev_private;
> +
> + writel(HIBMC_RAW_INTERRUPT_EN_VBLANK(1),
> + hidev->mmio + HIBMC_RAW_INTERRUPT_EN);
> +
> return 0;
> }
>
> static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> + struct hibmc_drm_device *hidev =
> + (struct hibmc_drm_device *)dev->dev_private;
> +
> + writel(HIBMC_RAW_INTERRUPT_EN_VBLANK(0),
> + hidev->mmio + HIBMC_RAW_INTERRUPT_EN);
> +}
> +
> +irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
> +{
> + struct drm_device *dev = (struct drm_device *)arg;
> + struct hibmc_drm_device *hidev =
> + (struct hibmc_drm_device *)dev->dev_private;
> + struct drm_crtc *crtc = &hidev->crtc;
> + u32 status;
> +
> + status = readl(hidev->mmio + HIBMC_RAW_INTERRUPT);
> +
> + if (status & HIBMC_RAW_INTERRUPT_VBLANK(1)) {
> + writel(HIBMC_RAW_INTERRUPT_VBLANK(1),
> + hidev->mmio + HIBMC_RAW_INTERRUPT);
> + drm_crtc_handle_vblank(crtc);
> + }
> +
> + return IRQ_HANDLED;
> }
>
> static struct drm_driver hibmc_driver = {
> .driver_features = DRIVER_GEM | DRIVER_MODESET |
> - DRIVER_ATOMIC,
> + DRIVER_ATOMIC | DRIVER_HAVE_IRQ,
> .fops = &hibmc_fops,
> .name = "hibmc",
> .date = "20160828",
> @@ -63,6 +93,7 @@ static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
> .dumb_create = hibmc_dumb_create,
> .dumb_map_offset = hibmc_dumb_mmap_offset,
> .dumb_destroy = drm_gem_dumb_destroy,
> + .irq_handler = hibmc_drm_interrupt,
> };
>
> static int hibmc_pm_suspend(struct device *dev)
> @@ -242,6 +273,13 @@ static int hibmc_unload(struct drm_device *dev)
> struct hibmc_drm_device *hidev = dev->dev_private;
>
> hibmc_fbdev_fini(hidev);
> +
> + if (dev->irq_enabled)
> + drm_irq_uninstall(dev);
> + if (hidev->msi_enabled)
> + pci_disable_msi(dev->pdev);
> + drm_vblank_cleanup(dev);
> +
> hibmc_kms_fini(hidev);
> hibmc_mm_fini(hidev);
> hibmc_hw_fini(hidev);
> @@ -272,6 +310,22 @@ static int hibmc_load(struct drm_device *dev, unsigned long flags)
> if (ret)
> goto err;
>
> + ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
> + if (ret) {
> + DRM_ERROR("failed to initialize vblank.\n");
> + goto err;
> + }
> +
> + hidev->msi_enabled = 0;
> + if (pci_enable_msi(dev->pdev)) {
It would be useful to check and print the return value of this.
> + DRM_ERROR("Enabling MSI failed!\n");
> + } else {
> + hidev->msi_enabled = 1;
> + ret = drm_irq_install(dev, dev->pdev->irq);
> + if (ret)
> + DRM_ERROR("install irq failed , ret = %d\n", ret);
DRM_WARN might be more appropriate, given that this isn't considered fatal.
> + }
> +
> /* reset all the states of crtc/plane/encoder/connector */
> drm_mode_config_reset(dev);
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 450247d..f1706fb 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -42,6 +42,7 @@ struct hibmc_drm_device {
> void __iomem *fb_map;
> unsigned long fb_base;
> unsigned long fb_size;
> + int msi_enabled;
Why not bool?
>
> /* drm */
> struct drm_device *dev;
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 9/9] MAINTAINERS: Update HISILICON DRM entries
From: Sean Paul @ 2016-11-11 1:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-10-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
Acked-by: Sean Paul <seanpaul@chromium.org>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c447953..cc5ee3a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4117,6 +4117,7 @@ F: drivers/gpu/drm/gma500/
>
> DRM DRIVERS FOR HISILICON
> M: Xinliang Liu <z.liuxinliang@hisilicon.com>
> +M: Rongrong Zou <zourongrong@gmail.com>
> R: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> R: Chen Feng <puck.chen@hisilicon.com>
> L: dri-devel at lists.freedesktop.org
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v9 2/8] power: add power sequence library
From: Peter Chen @ 2016-11-11 1:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJZ5v0ioqMqoaJqK0CTXafWRKoohmZSiOWKRgVZ9DR3_8+-YDw@mail.gmail.com>
On Fri, Nov 11, 2016 at 02:05:01AM +0100, Rafael J. Wysocki wrote:
> On Tue, Nov 8, 2016 at 3:51 AM, Peter Chen <peter.chen@nxp.com> wrote:
> > We have an well-known problem that the device needs to do some power
> > sequence before it can be recognized by related host, the typical
> > example like hard-wired mmc devices and usb devices.
> >
> > This power sequence is hard to be described at device tree and handled by
> > related host driver, so we have created a common power sequence
> > library to cover this requirement. The core code has supplied
> > some common helpers for host driver, and individual power sequence
> > libraries handle kinds of power sequence for devices. The pwrseq
> > librares always need to allocate extra instance for compatible
> > string match.
> >
> > pwrseq_generic is intended for general purpose of power sequence, which
> > handles gpios and clocks currently, and can cover other controls in
> > future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence is needed, else call of_pwrseq_on_list
> > /of_pwrseq_off_list instead (eg, USB hub driver).
> >
> > For new power sequence library, it can add its compatible string
> > to pwrseq_of_match_table, then the pwrseq core will match it with
> > DT's, and choose this library at runtime.
>
> In the first place, please document this stuff better than you have so
> far. To a minimum, add kerneldoc comments to all new non-trivial new
> functions to document what they are for and how they are expected to
> be used (especially the ones exported to drivers).
>
Thanks for your comments.
I will add kerneldoc for main APIs.
> Also, is there any guidance available for people who may want to use it?
No doc now, only some guidance in this commit log.
> > +config PWRSEQ_GENERIC
> > + bool "Generic power sequence control"
> > + depends on OF
> > + select POWER_SEQUENCE
> > + help
> > + It is used for drivers which needs to do power sequence
> > + (eg, turn on clock, toggle reset gpio) before the related
> > + devices can be found by hardware. This generic one can be
> > + used for common power sequence control.
>
> I wouldn't set it up this way.
>
> There are two problems here.
>
> First, say a distro is going to ship a multiplatform generic kernel.
> How they are going to figure out whether or not to set the new symbol
> in that kernel?
>
> Second, how users are supposed to know whether or not they will need
> it even if they build the kernel by themselves?
>
> It would be better IMO to set things up to select the new symbol from
> places making use of the code depending on it.
>
Will change it like below:
#
# Power Sequence library
#
menuconfig POWER_SEQUENCE
bool "Power sequence control"
depends on OF
help
It is used for drivers which needs to do power sequence
(eg, turn on clock, toggle reset gpio) before the related
devices can be found by hardware.
if POWER_SEQUENCE
config PWRSEQ_GENERIC
bool "Generic power sequence control"
default y
help
This is the generic power sequence control library, and is
supposed to support common power sequence usage.
endif
And the current user usb core will select POWER_SEQUENCE.
--
Best Regards,
Peter Chen
^ permalink raw reply
* [PATCH] pinctrl: sunxi: Free configs in pinctrl_map only if it is a config map
From: Chen-Yu Tsai @ 2016-11-11 2:35 UTC (permalink / raw)
To: linux-arm-kernel
In the recently refactored sunxi pinctrl library, we are only allocating
one set of pin configs for each pinmux setting node. When the pinctrl_map
structure is freed, the pin configs should also be freed. However the
code assumed the first map would contain the configs, which actually
never happens, as the mux function map gets added first.
The proper way to do this is to look through all the maps and free the
first one whose type is actually PIN_MAP_TYPE_CONFIGS_GROUP.
Also slightly expand the comment explaining this.
Fixes: f233dbca6227 ("pinctrl: sunxi: Rework the pin config building code")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index ebe2c73d211e..e199d95af8c0 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -408,8 +408,21 @@ static void sunxi_pctrl_dt_free_map(struct pinctrl_dev *pctldev,
struct pinctrl_map *map,
unsigned num_maps)
{
- /* All the maps have the same pin config, free only the first one */
- kfree(map[0].data.configs.configs);
+ int i;
+
+ /* pin config is never in the first map */
+ for (i = 1; i < num_maps; i++) {
+ if (map[i].type != PIN_MAP_TYPE_CONFIGS_GROUP)
+ continue;
+
+ /*
+ * All the maps share the same pin config,
+ * free only the first one we find.
+ */
+ kfree(map[i].data.configs.configs);
+ break;
+ }
+
kfree(map);
}
--
2.10.2
^ permalink raw reply related
* [PATCH v2 0/3] pinctrl: sunxi: Support generic pinconf functions
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi everyone,
This series fixes up generic pinconf support for the sunxi pinctrl driver
library. The driver was doing some bits wrong, like a) storing the pinconf
config value in its struct, and not actually reading the hardware to get
the current config, and b) not using the right arguments for the bias
parameters.
Patch 1 fixes the pin bias parameter arguments.
Patch 2 makes the driver read out pinconf settings from the hardware, and
returns the correct value for unsupported features and disable features.
With this in place it also declares itself as generic pinconf compatible,
which enables us to read the config through the debugfs pinconf interface.
Patch 3 makes the sunxi_pconf_group_set callback use the helper function
introduced in patch 1.
Changes since v1:
- Rebased onto the updated sunxi pinctrl driver with support for the
generic pinconf bindings
- Use separate value for what is written to the register in the pinconf
set function, as Maxime requested.
Regards
ChenYu
Chen-Yu Tsai (3):
pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument
pinctrl: sunxi: Add support for fetching pinconf settings from
hardware
pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 156 +++++++++++++++++++++++++---------
drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 -
2 files changed, 118 insertions(+), 39 deletions(-)
--
2.10.2
^ permalink raw reply
* [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN, UP} argument
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111024455.16883-1-wens@csie.org>
According to pinconf-generic.h, the argument for
PIN_CONFIG_BIAS_PULL_{DOWN,UP} is non-zero if the bias is enabled
with a pull up/down resistor, zero if it is directly connected
to VDD or ground.
Since Allwinner hardware uses a weak pull resistor internally,
the argument should be 1.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index e199d95af8c0..e04edda8629d 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -291,12 +291,16 @@ static unsigned long *sunxi_pctrl_build_pin_config(struct device_node *node,
if (sunxi_pctrl_has_bias_prop(node)) {
int pull = sunxi_pctrl_parse_bias_prop(node);
+ int arg = 0;
if (pull < 0) {
ret = pull;
goto err_free;
}
- pinconfig[idx++] = pinconf_to_config_packed(pull, 0);
+ if (pull != PIN_CONFIG_BIAS_DISABLE)
+ arg = 1; /* hardware uses weak pull resistors */
+
+ pinconfig[idx++] = pinconf_to_config_packed(pull, arg);
}
--
2.10.2
^ permalink raw reply related
* [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111024455.16883-1-wens@csie.org>
The sunxi pinctrl driver only caches whatever pinconf setting was last
set on a given pingroup. This is not particularly helpful, nor is it
correct.
Fix this by actually reading the hardware registers and returning
the correct results or error codes. Also filter out unsupported
pinconf settings. Since this driver has a peculiar setup of 1 pin
per group, we can support both pin and pingroup pinconf setting
read back with the same code. The sunxi_pconf_reg helper and code
structure is inspired by pinctrl-msm.
With this done we can also claim to support generic pinconf, by
setting .is_generic = true in pinconf_ops.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 84 +++++++++++++++++++++++++++++++++--
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index e04edda8629d..3e9f7c675d36 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -438,15 +438,91 @@ static const struct pinctrl_ops sunxi_pctrl_ops = {
.get_group_pins = sunxi_pctrl_get_group_pins,
};
+static int sunxi_pconf_reg(unsigned pin, enum pin_config_param param,
+ u32 *offset, u32 *shift, u32 *mask)
+{
+ switch (param) {
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ *offset = sunxi_dlevel_reg(pin);
+ *shift = sunxi_dlevel_offset(pin);
+ *mask = DLEVEL_PINS_MASK;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_UP:
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ case PIN_CONFIG_BIAS_DISABLE:
+ *offset = sunxi_pull_reg(pin);
+ *shift = sunxi_pull_offset(pin);
+ *mask = PULL_PINS_MASK;
+ break;
+
+ default:
+ return -ENOTSUPP;
+ }
+
+ return 0;
+}
+
+static int sunxi_pconf_get(struct pinctrl_dev *pctldev, unsigned pin,
+ unsigned long *config)
+{
+ struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param param = pinconf_to_config_param(*config);
+ u32 offset, shift, mask, val;
+ u16 arg;
+ int ret;
+
+ pin -= pctl->desc->pin_base;
+
+ ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
+ if (ret < 0)
+ return ret;
+
+ val = (readl(pctl->membase + offset) >> shift) & mask;
+
+ switch (pinconf_to_config_param(*config)) {
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ arg = (val + 1) * 10;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_UP:
+ if (val != SUN4I_PINCTRL_PULL_UP)
+ return -EINVAL;
+ arg = 1; /* hardware is weak pull-up */
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ if (val != SUN4I_PINCTRL_PULL_DOWN)
+ return -EINVAL;
+ arg = 1; /* hardware is weak pull-down */
+ break;
+
+ case PIN_CONFIG_BIAS_DISABLE:
+ if (val != SUN4I_PINCTRL_NO_PULL)
+ return -EINVAL;
+ arg = 0;
+ break;
+
+ default:
+ /* sunxi_pconf_reg should catch anything unsupported */
+ WARN_ON(1);
+ return -ENOTSUPP;
+ }
+
+ *config = pinconf_to_config_packed(param, arg);
+
+ return 0;
+}
+
static int sunxi_pconf_group_get(struct pinctrl_dev *pctldev,
unsigned group,
unsigned long *config)
{
struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct sunxi_pinctrl_group *g = &pctl->groups[group];
- *config = pctl->groups[group].config;
-
- return 0;
+ /* We only support 1 pin per group. Chain it to the pin callback */
+ return sunxi_pconf_get(pctldev, g->pin, config);
}
static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
@@ -518,6 +594,8 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
}
static const struct pinconf_ops sunxi_pconf_ops = {
+ .is_generic = true,
+ .pin_config_get = sunxi_pconf_get,
.pin_config_group_get = sunxi_pconf_group_get,
.pin_config_group_set = sunxi_pconf_group_set,
};
--
2.10.2
^ permalink raw reply related
* [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111024455.16883-1-wens@csie.org>
The sunxi_pconf_reg helper introduced in the last patch gives us the
chance to rework sunxi_pconf_group_set to have it match the structure
of sunxi_pconf_(group_)get and make it easier to understand.
For each config to set, it:
1. checks if the parameter is supported.
2. checks if the argument is within limits.
3. converts argument to the register value.
4. writes to the register with spinlock held.
As a result the function now blocks unsupported config parameters,
instead of silently ignoring them.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 66 +++++++++++++++++------------------
drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 -
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 3e9f7c675d36..fa11a3100346 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -532,23 +532,27 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
{
struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
struct sunxi_pinctrl_group *g = &pctl->groups[group];
- unsigned long flags;
unsigned pin = g->pin - pctl->desc->pin_base;
- u32 val, mask;
- u16 strength;
- u8 dlevel;
int i;
- spin_lock_irqsave(&pctl->lock, flags);
-
for (i = 0; i < num_configs; i++) {
- switch (pinconf_to_config_param(configs[i])) {
+ enum pin_config_param param;
+ unsigned long flags;
+ u32 offset, shift, mask, reg;
+ u16 arg, val;
+ int ret;
+
+ param = pinconf_to_config_param(configs[i]);
+ arg = pinconf_to_config_argument(configs[i]);
+
+ ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
+ if (ret < 0)
+ return ret;
+
+ switch (param) {
case PIN_CONFIG_DRIVE_STRENGTH:
- strength = pinconf_to_config_argument(configs[i]);
- if (strength > 40) {
- spin_unlock_irqrestore(&pctl->lock, flags);
+ if (arg < 10 || arg > 40)
return -EINVAL;
- }
/*
* We convert from mA to what the register expects:
* 0: 10mA
@@ -556,39 +560,33 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
* 2: 30mA
* 3: 40mA
*/
- dlevel = strength / 10 - 1;
- val = readl(pctl->membase + sunxi_dlevel_reg(pin));
- mask = DLEVEL_PINS_MASK << sunxi_dlevel_offset(pin);
- writel((val & ~mask)
- | dlevel << sunxi_dlevel_offset(pin),
- pctl->membase + sunxi_dlevel_reg(pin));
+ val = arg / 10 - 1;
break;
case PIN_CONFIG_BIAS_DISABLE:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask),
- pctl->membase + sunxi_pull_reg(pin));
+ val = 0;
break;
case PIN_CONFIG_BIAS_PULL_UP:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask) | 1 << sunxi_pull_offset(pin),
- pctl->membase + sunxi_pull_reg(pin));
+ if (arg == 0)
+ return -EINVAL;
+ val = 1;
break;
case PIN_CONFIG_BIAS_PULL_DOWN:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask) | 2 << sunxi_pull_offset(pin),
- pctl->membase + sunxi_pull_reg(pin));
+ if (arg == 0)
+ return -EINVAL;
+ val = 2;
break;
default:
- break;
+ /* sunxi_pconf_reg should catch anything unsupported */
+ WARN_ON(1);
+ return -ENOTSUPP;
}
- /* cache the config value */
- g->config = configs[i];
- } /* for each config */
- spin_unlock_irqrestore(&pctl->lock, flags);
+ spin_lock_irqsave(&pctl->lock, flags);
+ reg = readl(pctl->membase + offset);
+ reg &= ~(mask << shift);
+ writel(reg | val << shift, pctl->membase + offset);
+ spin_unlock_irqrestore(&pctl->lock, flags);
+ } /* for each config */
return 0;
}
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 0afce1ab12d0..a7efb31d6523 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -109,7 +109,6 @@ struct sunxi_pinctrl_function {
struct sunxi_pinctrl_group {
const char *name;
- unsigned long config;
unsigned pin;
};
--
2.10.2
^ permalink raw reply related
* [PATCH v27 1/9] memblock: add memblock_cap_memory_range()
From: AKASHI Takahiro @ 2016-11-11 2:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110172720.GB17134@arm.com>
Will,
(+ Cc: Dennis)
On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > Add memblock_cap_memory_range() which will remove all the memblock regions
> > except the range specified in the arguments.
> >
> > This function, like memblock_mem_limit_remove_map(), will not remove
> > memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
> > later as "device memory."
> > See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
> > address the mem limit issue").
> >
> > This function is used, in a succeeding patch in the series of arm64 kdump
> > suuport, to limit the range of usable memory, System RAM, on crash dump
> > kernel.
> > (Please note that "mem=" parameter is of little use for this purpose.)
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Cc: linux-mm at kvack.org
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > include/linux/memblock.h | 1 +
> > mm/memblock.c | 28 ++++++++++++++++++++++++++++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index 5b759c9..0e770af 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
> > phys_addr_t memblock_end_of_DRAM(void);
> > void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > void memblock_mem_limit_remove_map(phys_addr_t limit);
> > +void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> > bool memblock_is_memory(phys_addr_t addr);
> > int memblock_is_map_memory(phys_addr_t addr);
> > int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index 7608bc3..eb53876 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
> > (phys_addr_t)ULLONG_MAX);
> > }
> >
> > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > +{
> > + int start_rgn, end_rgn;
> > + int i, ret;
> > +
> > + if (!size)
> > + return;
> > +
> > + ret = memblock_isolate_range(&memblock.memory, base, size,
> > + &start_rgn, &end_rgn);
> > + if (ret)
> > + return;
> > +
> > + /* remove all the MAP regions */
> > + for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > + memblock_remove_region(&memblock.memory, i);
> > +
> > + for (i = start_rgn - 1; i >= 0; i--)
> > + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > + memblock_remove_region(&memblock.memory, i);
> > +
> > + /* truncate the reserved regions */
> > + memblock_remove_range(&memblock.reserved, 0, base);
> > + memblock_remove_range(&memblock.reserved,
> > + base + size, (phys_addr_t)ULLONG_MAX);
> > +}
>
> This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> you not implement that in terms of your new, more general, function? e.g.
> by passing base == 0, and size == limit?
Obviously it's possible.
I actually talked to Dennis before about merging them,
but he was against my idea.
Thanks,
-Takahiro AKASHI
> Will
^ 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