[PATCHv2] ARM: exynos_defconfig: Enable CONFIG_SND_SOC_ODROIDX2 for Odroid-XU3
Enable CONFIG_SND_SOC_ODROIDX2 and CONFIG_SND_SIMPLE_CARD to enable sound on Odroid-XU3 board using the max98090 audio codec. Signed-off-by: Anand Moon Reviewed-by: Lukasz Majewski --- Changes v2: Fixed the commit log. Signed-off-by: Anand Moon --- arch/arm/configs/exynos_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig index 9504e77..1b7253b 100644 --- a/arch/arm/configs/exynos_defconfig +++ b/arch/arm/configs/exynos_defconfig @@ -144,6 +144,8 @@ CONFIG_SND=y CONFIG_SND_SOC=y CONFIG_SND_SOC_SAMSUNG=y CONFIG_SND_SOC_SNOW=y +CONFIG_SND_SOC_ODROIDX2=y +CONFIG_SND_SIMPLE_CARD=y CONFIG_USB=y CONFIG_USB_ANNOUNCE_NEW_DEVICES=y CONFIG_USB_XHCI_HCD=y -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] drm/exynos: ipp: validate a GEM handle with multiple planes
FIMC & GSC driver can calculate the offset of planes. So there are use cases which IPP receives just one GEM handle of an image with multiple plane. This patch extends ipp_validate_mem_node() to validate this case. Signed-off-by: Hyungwon Hwang --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 51 - 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 54c5cf4..b3dc778 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -482,8 +482,8 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev, { struct drm_exynos_ipp_config *ipp_cfg; unsigned int num_plane; - unsigned long min_size, size; - unsigned int bpp; + unsigned long size, buf_size = 0, plane_size, img_size = 0; + unsigned int bpp, width, height; int i; ipp_cfg = &c_node->property.config[m_node->ops_id]; @@ -497,20 +497,45 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev, * but it seems more than enough */ for (i = 0; i < num_plane; ++i) { - if (!m_node->buf_info.handles[i]) { - DRM_ERROR("invalid handle for plane %d\n", i); - return -EINVAL; - } + width = ipp_cfg->sz.hsize; + height = ipp_cfg->sz.vsize; bpp = drm_format_plane_cpp(ipp_cfg->fmt, i); - min_size = (ipp_cfg->sz.hsize * ipp_cfg->sz.vsize * bpp) >> 3; - size = exynos_drm_gem_get_size(drm_dev, - m_node->buf_info.handles[i], - c_node->filp); - if (min_size > size) { - DRM_ERROR("invalid size for plane %d\n", i); - return -EINVAL; + + /* +* The result of drm_format_plane_cpp() for chroma planes must +* be used with drm_format__chroma_subsampling() for +* correct result. +*/ + if (i > 0) { + width /= drm_format_horz_chroma_subsampling( + ipp_cfg->fmt); + height /= drm_format_vert_chroma_subsampling( + ipp_cfg->fmt); } + plane_size = width * height * bpp; + img_size += plane_size; + + if (m_node->buf_info.handles[i]) { + size = exynos_drm_gem_get_size(drm_dev, + m_node->buf_info.handles[i], + c_node->filp); + if (plane_size > size) { + DRM_ERROR( + "buffer %d is smaller than required\n", + i); + return -EINVAL; + } + + buf_size += size; + } + } + + if (buf_size < img_size) { + DRM_ERROR("size of buffers(%lu) is smaller than image(%lu)\n", + buf_size, img_size); + return -EINVAL; } + return 0; } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] drm/exynos: ipp: fix wrong index referencing a config element
Config depends on the opreation. So it must be referenced by an operation id, not a property id. Signed-off-by: Hyungwon Hwang --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index b7f1cbc..54c5cf4 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -486,8 +486,7 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev, unsigned int bpp; int i; - /* The property id should already be varified */ - ipp_cfg = &c_node->property.config[m_node->prop_id]; + ipp_cfg = &c_node->property.config[m_node->ops_id]; num_plane = drm_format_num_planes(ipp_cfg->fmt); /** -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] drm/exynos: dsi: check whether dsi is enabled before sending data
exynos_dsi_host_transfer() can be called through a panel driver while DSI is turning down. It is possible because the function checks only whether DSI is initialized or not, and there is a moment which DSI is set by uninitialized, but DSI is still turning down. To prevent it, DSI must be set by disabled before starting to be turned down, and exynos_dsi_host_transfer() must check whether DSI is enabled or not. Kernel dump: [ 4721.351448] Unhandled fault: synchronous external abort (0x96000210) at 0xff800015e018 [ 4721.351809] Internal error: : 96000210 [#1] PREEMPT SMP [ 4721.352031] Modules linked in: [ 4721.352173] CPU: 2 PID: 300 Comm: deviced Tainted: GW 4.0.4-01017-g7964a87 #1 [ 4721.353989] Hardware name: Samsung DRACO board (DT) [ 4721.358852] task: ffc0a0b7 ti: ffc0a00ec000 task.ti: ffc0a00ec000 [ 4721.366327] PC is at exynos_dsi_enable_lane+0x14/0x5c [ 4721.371353] LR is at exynos_dsi_host_transfer+0x834/0x8d8 [ 4721.376731] pc : [] lr : [] pstate: 6145 [ 4721.384107] sp : ffc0a00efbe0 [ 4721.387405] x29: ffc0a00efbe0 x28: ffc0a00ec000 [ 4721.392699] x27: ffc000968000 x26: 0040 [ 4721.397994] x25: ffc000f74dc0 x24: ffc0a00efec8 [ 4721.403290] x23: ffc0a4815400 x22: ffc0009f2729 [ 4721.408584] x21: ffc0a00efcc8 x20: ffc0a4a2a848 [ 4721.413879] x19: ffc0a4a2a818 x18: 0004 [ 4721.419173] x17: 007faa5cddf0 x16: ffc0001a40a8 [ 4721.424469] x15: 0009 x14: 000d [ 4721.429762] x13: 6e6e6f63206b726f x12: 0010 [ 4721.435058] x11: 0101010101010101 x10: [ 4721.440353] x9 : 000a x8 : 8386838282818381 [ 4721.445648] x7 : ffc0a201efe8 x6 : [ 4721.450943] x5 : fffa x4 : ffc0a201f170 [ 4721.456237] x3 : ff800015e000 x2 : ff800015e018 [ 4721.461531] x1 : 000f x0 : ffc0a4a2a818 [ 4721.466826] [ 4721.468305] Process deviced (pid: 300, stack limit = 0xffc0a00ec028) [ 4721.474989] Stack: (0xffc0a00efbe0 to 0xffc0a00f) [ 4721.480720] fbe0: a00efca0 ffc0 0042c944 ffc0 a0f2d680 ffc0 0024 [ 4721.488895] fc00: a4b6d000 ffc0 009f2729 ffc0 a4815400 ffc0 a00efec8 ffc0 Signed-off-by: Hyungwon Hwang --- Changes for v2: - Previous version of this patch makes a problem in initializing the DSI. This patch fixes it, by moving the point which DSI is set by enabled to the point before drm_panel_prepare() called. This is where the setting must be done, because to call drm_panel_prepare() successfully, DSI must be enabled. Also this patch adds the condition to TE irq handler. DSI must be enabled and initialized, not just enabled before calling te_handler in the display driver. Changes for v3: - New state DSIM_STATE_VIDOUT_AVAILABLE for representing video output availability is introduced. Because DSIM_STATE_ENABLED becomes to represent whether DSI can be used for data transfer or not, this state can be used for checking whether video ouput is available or not anymore. So new state have to be introduced. The stete DSIM_STATE_VIDOUT_AVAILABLE represents whether DSI is prepared for outputting video to a panel or not. drivers/gpu/drm/exynos/exynos_drm_dsi.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 1cfc4be07..9250356 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -241,6 +241,7 @@ struct exynos_dsi_transfer { #define DSIM_STATE_ENABLED BIT(0) #define DSIM_STATE_INITIALIZED BIT(1) #define DSIM_STATE_CMD_LPM BIT(2) +#define DSIM_STATE_VIDOUT_AVAILABLEBIT(3) struct exynos_dsi_driver_data { unsigned int *regs; @@ -1268,7 +1269,7 @@ static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id) struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id; struct drm_encoder *encoder = dsi->display.encoder; - if (dsi->state & DSIM_STATE_ENABLED) + if (dsi->state & DSIM_STATE_VIDOUT_AVAILABLE) exynos_drm_crtc_te_handler(encoder->crtc); return IRQ_HANDLED; @@ -1408,6 +1409,9 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host, struct exynos_dsi_transfer xfer; int ret; + if (!(dsi->state & DSIM_STATE_ENABLED)) + return -EINVAL; + if (!(dsi->state & DSIM_STATE_INITIALIZED)) { ret = exynos_dsi_init(dsi); if (ret) @@ -1520,8 +1524,11 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi) if (ret < 0) return ret; + dsi->state |= DSIM_STATE_ENABLED; + ret = drm_panel_prepare(dsi->panel); if (ret < 0) { + dsi->state &= ~DSIM_STATE_ENABLED; exynos_dsi_poweroff(ds
Re: [PATCHv2 2/2] ARM: exynos_defconfig: Enable CONFIG_LEDS_PWM for Odroid-XU3
Hi On 9 June 2015 at 05:26, Krzysztof Kozlowski wrote: > On 08.06.2015 02:03, Anand Moon wrote: >> Enable CONFIG_LEDS_PWM amd CONFIG_LEDS_TRIGGER_HEARTBEAT for >> Odroid-XU3 board. > > I am a little confused. > > This is v2. > > I found version 3, a little different: > http://www.spinics.net/lists/linux-samsung-soc/msg44406.html > (which > > Where is the changelog? Which one is newer? Should we drop the v3? > While at it, can you add information WHY do you want to enable LEDS? > > Best regards, > Krzysztof > Look like I an doom to make mistake. With the introduction of this http://www.spinics.net/lists/linux-samsung-soc/msg44481.html We need to enable CONFIG_LEDS_PWM to make it working. So please drop the v3 patch. -Anand Moon -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] ARM: exynos_defconfig: Enable CONFIG_SND_SOC_ODROIDX2 for Odroid-XU3
Hi, On 9 June 2015 at 05:20, Krzysztof Kozlowski wrote: > On 08.06.2015 02:03, Anand Moon wrote: >> Enable CONFIG_SND_SOC_ODROIDX2 and CONFIG_SND_SIMPLE_CARD to enable >> sound on Odroid-XU3 board. >> >> Below is the output of boot log. >> [6.021550] max98090 5-0010: MAX98090 REVID=0x43 >> [6.025506] random: nonblocking pool is initialized >> [6.047297] asoc-simple-card sound: HiFi <-> 383.i2s mapping ok >> [6.068392] s5m-rtc s2mps14-rtc: setting system clock to 2015-06-07 >> 12:51:06 UTC (1433681466) >> [6.123650] ALSA device list: >> [6.135203] #0: Odroid-XU3 > > Answering the question "why" and "what is device" would be sufficient. > You answered the first question ("enable sound") so you can just add > second part - using the max98090 audio codec. > > This whole dmesg does not bring any benefits here, especially the > irrelevant items (like random or RTC). Could you trim the commit message? > > Beside of that it looks fine however you marked this as "v2": > 1. I cannot find v1 on Google. The title was the same? > 2. Where is changelog? > > Best regards, > Krzysztof Look like I an doom to make mistake. This series of patch got PATCHv2 tag, It's my mistake. not maintain the version of the patch. I will resend it. -Anand Moon -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 2/2] ARM: exynos_defconfig: Enable CONFIG_LEDS_PWM for Odroid-XU3
On 08.06.2015 02:03, Anand Moon wrote: > Enable CONFIG_LEDS_PWM amd CONFIG_LEDS_TRIGGER_HEARTBEAT for > Odroid-XU3 board. I am a little confused. This is v2. I found version 3, a little different: http://www.spinics.net/lists/linux-samsung-soc/msg44406.html (which Where is the changelog? Which one is newer? Should we drop the v3? While at it, can you add information WHY do you want to enable LEDS? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] ARM: exynos_defconfig: Enable CONFIG_SND_SOC_ODROIDX2 for Odroid-XU3
On 08.06.2015 02:03, Anand Moon wrote: > Enable CONFIG_SND_SOC_ODROIDX2 and CONFIG_SND_SIMPLE_CARD to enable > sound on Odroid-XU3 board. > > Below is the output of boot log. > [6.021550] max98090 5-0010: MAX98090 REVID=0x43 > [6.025506] random: nonblocking pool is initialized > [6.047297] asoc-simple-card sound: HiFi <-> 383.i2s mapping ok > [6.068392] s5m-rtc s2mps14-rtc: setting system clock to 2015-06-07 > 12:51:06 UTC (1433681466) > [6.123650] ALSA device list: > [6.135203] #0: Odroid-XU3 Answering the question "why" and "what is device" would be sufficient. You answered the first question ("enable sound") so you can just add second part - using the max98090 audio codec. This whole dmesg does not bring any benefits here, especially the irrelevant items (like random or RTC). Could you trim the commit message? Beside of that it looks fine however you marked this as "v2": 1. I cannot find v1 on Google. The title was the same? 2. Where is changelog? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cpufreq:exynos-cpufreq - Fix for memory leak in case SOC name does not match.
On Monday, June 08, 2015 10:31:00 AM Lukasz Majewski wrote: > Hi Viresh, > > > On 25-05-15, 07:39, Shailendra Verma wrote: > > > During probe free the memory allocated to "exynos_info" in case of > > > unknown SOC type. > > > > > > Signed-off-by: Shailendra Verma > > > --- > > > drivers/cpufreq/exynos-cpufreq.c |6 -- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/cpufreq/exynos-cpufreq.c > > > b/drivers/cpufreq/exynos-cpufreq.c index 82d2fbb..8682378 100644 > > > --- a/drivers/cpufreq/exynos-cpufreq.c > > > +++ b/drivers/cpufreq/exynos-cpufreq.c > > > @@ -182,7 +182,7 @@ static int exynos_cpufreq_probe(struct > > > platform_device *pdev) ret = exynos5250_cpufreq_init(exynos_info); > > > } else { > > > pr_err("%s: Unknown SoC type\n", __func__); > > > - return -ENODEV; > > > + ret = -ENODEV; > > > } > > > > > > if (ret) > > > @@ -190,12 +190,14 @@ static int exynos_cpufreq_probe(struct > > > platform_device *pdev) > > > if (exynos_info->set_freq == NULL) { > > > dev_err(&pdev->dev, "No set_freq function > > > (ERR)\n"); > > > + ret = -EINVAL; > > > goto err_vdd_arm; > > > } > > > > > > arm_regulator = regulator_get(NULL, "vdd_arm"); > > > if (IS_ERR(arm_regulator)) { > > > dev_err(&pdev->dev, "failed to get resource > > > vdd_arm\n"); > > > + ret = -EINVAL; > > > goto err_vdd_arm; > > > } > > > > > > @@ -227,7 +229,7 @@ err_cpufreq_reg: > > > regulator_put(arm_regulator); > > > err_vdd_arm: > > > kfree(exynos_info); > > > - return -EINVAL; > > > + return ret; > > > } > > > > > > static struct platform_driver exynos_cpufreq_platdrv = { > > > > Acked-by: Viresh Kumar > > > > Acked-by: Lukasz Majewski I'm assuming that it will go it through the Samsung tree. -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: ARM: EXYNOS: Describe boot loaders interface
On Sat, 6 Jun 2015 19:05:51 +0900 Krzysztof Kozlowski wrote: > The document, based solely on kernel source code, tries to group the > information scattered over different files. This would help in the > future when adding support for new SoC or when extending features > related to low power modes. I've applied this to the docs tree, thanks. jon -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 6/8] mfd: cros_ec: Support multiple EC in a system
On Fri, Jun 05, 2015 at 11:17:30AM +0100, Lee Jones wrote: > On Thu, 04 Jun 2015, Javier Martinez Canillas wrote: > > > From: Gwendal Grignou > > > > Chromebooks can have more than one Embedded Controller so the > > cros_ec device id has to be incremented for each EC registered. > > > > Add a new structure to represent multiple EC as different char > > devices (e.g: /dev/cros_ec, /dev/cros_pd). It connects to > > cros_ec_device and allows sysfs inferface for cros_pd. > > > > Also reduce number of allocated objects, make chromeos sysfs > > class object a static and add refcounting to prevent object > > deletion while command is in progress. > > > > Signed-off-by: Gwendal Grignou > > Reviewed-by: Dmitry Torokhov > > Signed-off-by: Javier Martinez Canillas > > Tested-by: Heiko Stuebner > > --- > > > > Changes since v5: > > - Don't allow to change the device name from DT. Suggested by Lee Jones. > > - Expand error messages in case of mfd_add_devices() failure. > >Suggested by Lee Jones. > > > > Changes since v4: > > - Use cros-ec-name DT property instead of devname. Suggested by Lee Jones. > > - Pass PLATFORM_DEVID_AUTO directly to mfd_add_devices(). > >Suggested by Lee Jones. > > - Add Heiko Stuebner tested-by tag. > > - Fix get_version by passing the cmd_offset to EC_CMD_GET_VERSION. > > > > Changes since v3: > > - Add defines for the EC and PD index constants. > > - Remove cros_ec_dev_register() and declare the mfd_cells as static > > structs. > >Suggested by Lee Jones. > > - Add a new line before the return statement in cros_ec_dev_register(). > >Suggested by Lee Jones. > > > > Changes since v2: None > > > > Changes since v1: > > - Squash patch that adds support to represent EC's as different > > char devices (e.g: /dev/cros_ec, /dev/cros_pd): > > https://chromium-review.googlesource.com/#/c/217297/ > > Suggested by Gwendal Grignou > > - Use cros_ec instead of cros-ec in the subject line to be consistent. > > Suggested by Gwendal Grignou > > --- > > drivers/input/keyboard/cros_ec_keyb.c | 2 +- > > drivers/mfd/cros_ec.c | 52 ++-- > > drivers/mfd/cros_ec_i2c.c | 1 - > > drivers/mfd/cros_ec_spi.c | 1 - > > drivers/platform/chrome/cros_ec_dev.c | 130 > > - > > drivers/platform/chrome/cros_ec_dev.h | 7 -- > > drivers/platform/chrome/cros_ec_lightbar.c | 75 + > > drivers/platform/chrome/cros_ec_lpc.c | 1 - > > drivers/platform/chrome/cros_ec_sysfs.c| 48 +-- > > include/linux/mfd/cros_ec.h| 44 -- > > 10 files changed, 234 insertions(+), 127 deletions(-) > > For my own reference: > Acked-by: Lee Jones > > Let me know when you have all the appropriate Acks and I'll apply the > set. Whole series: Acked-by: Olof Johansson I'm OK with this going through the mfd tree, since there's nothing queued up for chrome-platform that this would conflict with. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 08.06.2015 um 20:14 schrieb Alexander Holler: Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult: Am 04.06.2015 um 22:39 schrieb Alexander Holler: > As it seems to have been forgotten or overread, I've mentioned in my series of patches last year that, with a few changes, it's possible to let the algorithm I've used (dfs) to spit out all drivers which can be initialized in parallel. Unfortunately, I've missed that ... could you please resend you patches? Boot time reduction is one of the topics on my 2do in several weeks. https://lkml.org/lkml/2014/5/12/452 And don't forget patch 10/9 which fixed a bug in my previous patch series and which alos was the reason for the large difference in boot times with and without deps: https://lkml.org/lkml/2014/5/13/567 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult: Am 04.06.2015 um 22:39 schrieb Alexander Holler: > As it seems to have been forgotten or overread, I've mentioned in my series of patches last year that, with a few changes, it's possible to let the algorithm I've used (dfs) to spit out all drivers which can be initialized in parallel. Unfortunately, I've missed that ... could you please resend you patches? Boot time reduction is one of the topics on my 2do in several weeks. https://lkml.org/lkml/2014/5/12/452 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] thermal: exynos: Disable the regulator on probe failure
Hi Krzysztof, > During probe the regulator (if present) was enabled but not disabled > in case of failure. So an unsuccessful probe lead to enabling the > regulator which was actually not needed because the device was not > enabled. > > Additionally each deferred probe lead to increase of regulator enable > count so it would not be effectively disabled during removal of the > device. Thanks for catching this. > > Signed-off-by: Krzysztof Kozlowski > Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator > defined at device tree") Cc: > > --- > > I am not entirely convinced that this should go to stable. Leaving a > regulator enabled in case of probe failure (no exynos TMU device) or > after deferred probe (regulator won't be disabled during device > removal) is not a critical issue, just leaks power. > --- > drivers/thermal/samsung/exynos_tmu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/thermal/samsung/exynos_tmu.c > b/drivers/thermal/samsung/exynos_tmu.c index > 531f4b179871..13c3aceed19d 100644 --- > a/drivers/thermal/samsung/exynos_tmu.c +++ > b/drivers/thermal/samsung/exynos_tmu.c @@ -1392,6 +1392,8 @@ > err_clk_sec: if (!IS_ERR(data->clk_sec)) > clk_unprepare(data->clk_sec); > err_sensor: > + if (!IS_ERR_OR_NULL(data->regulator)) > + regulator_disable(data->regulator); > thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); > > return ret; Acked-by: Lukasz Majewski I will test it and afterwards add to samsung-thermal tree. -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] ARM: exynos_defconfig: Enable CONFIG_SND_SOC_ODROIDX2 for Odroid-XU3
Hi Anand, > Enable CONFIG_SND_SOC_ODROIDX2 and CONFIG_SND_SIMPLE_CARD to enable > sound on Odroid-XU3 board. > > Below is the output of boot log. > [6.021550] max98090 5-0010: MAX98090 REVID=0x43 > [6.025506] random: nonblocking pool is initialized > [6.047297] asoc-simple-card sound: HiFi <-> 383.i2s mapping ok > [6.068392] s5m-rtc s2mps14-rtc: setting system clock to > 2015-06-07 12:51:06 UTC (1433681466) [6.123650] ALSA device list: > [6.135203] #0: Odroid-XU3 > > Signed-off-by: Anand Moon > --- > arch/arm/configs/exynos_defconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/configs/exynos_defconfig > b/arch/arm/configs/exynos_defconfig index 9504e77..1b7253b 100644 > --- a/arch/arm/configs/exynos_defconfig > +++ b/arch/arm/configs/exynos_defconfig > @@ -144,6 +144,8 @@ CONFIG_SND=y > CONFIG_SND_SOC=y > CONFIG_SND_SOC_SAMSUNG=y > CONFIG_SND_SOC_SNOW=y > +CONFIG_SND_SOC_ODROIDX2=y > +CONFIG_SND_SIMPLE_CARD=y > CONFIG_USB=y > CONFIG_USB_ANNOUNCE_NEW_DEVICES=y > CONFIG_USB_XHCI_HCD=y Reviewed-by: Lukasz Majewski -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] usb: ehci-exynos: Make provision for vdd regulators
On Mon, 8 Jun 2015, Vivek Gautam wrote: > Facilitate getting required 3.3V and 1.0V VDD supply for > EHCI controller on Exynos. > > For example, patches for regulators' nodes: > c8c253f ARM: dts: Add regulator entries to smdk5420 > 275dcd2 ARM: dts: add max77686 pmic node for smdk5250, > enable only minimum number of regulators on smdk5250. > > So ensuring now that the controller driver requests the necessary > VDD regulators (if available, unless there are direct VDD rails), > and enable them so as to make them working on exynos systems. > > Signed-off-by: Vivek Gautam Something about this looks a little fishy... > @@ -170,7 +173,27 @@ static int exynos_ehci_probe(struct platform_device > *pdev) > > err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci); > if (err) > - goto fail_clk; > + goto fail_regulator1; > + > + exynos_ehci->vdd33 = devm_regulator_get_optional(&pdev->dev, "vdd33"); Just before this region of code, there is: if (of_device_is_compatible(pdev->dev.of_node, "samsung,exynos5440-ehci")) goto skip_phy; If that "goto" is taken, exynos_ehci->vdd33 and ->vdd10 will be NULL, not an ERR_PTR code. > + if (!IS_ERR(exynos_ehci->vdd33)) { > + err = regulator_enable(exynos_ehci->vdd33); > + if (err) { > + dev_err(&pdev->dev, > + "Failed to enable 3.3V Vdd supply\n"); > + goto fail_regulator1; > + } > + } > + > + exynos_ehci->vdd10 = devm_regulator_get_optional(&pdev->dev, "vdd10"); > + if (!IS_ERR(exynos_ehci->vdd10)) { > + err = regulator_enable(exynos_ehci->vdd10); > + if (err) { > + dev_err(&pdev->dev, > + "Failed to enable 1.0V Vdd supply\n"); > + goto fail_regulator2; > + } > + } > > skip_phy: > > @@ -231,6 +254,12 @@ fail_add_hcd: > fail_io: > clk_disable_unprepare(exynos_ehci->clk); > fail_clk: > + if (!IS_ERR(exynos_ehci->vdd10)) > + regulator_disable(exynos_ehci->vdd10); > +fail_regulator2: > + if (!IS_ERR(exynos_ehci->vdd33)) > + regulator_disable(exynos_ehci->vdd33); Which means these regulator_disable() calls will crash when they dereference a NULL pointer. I think it would be simpler in the end to let a NULL pointer mean the regulator isn't present. If devm_regulator_get_optional() returns an error, convert it to NULL (or don't do the assignment to exynos_ehci->vdd?? in the first place). The same criticism applies to the other patch in this series. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 2/2] ARM: exynos_defconfig: Enable CONFIG_LEDS_PWM for Odroid-XU3
Hi Anand, > Enable CONFIG_LEDS_PWM amd CONFIG_LEDS_TRIGGER_HEARTBEAT for > Odroid-XU3 board. > > Signed-off-by: Anand Moon > --- > arch/arm/configs/exynos_defconfig | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/configs/exynos_defconfig > b/arch/arm/configs/exynos_defconfig index 1b7253b..2bbdf80 100644 > --- a/arch/arm/configs/exynos_defconfig > +++ b/arch/arm/configs/exynos_defconfig > @@ -165,6 +165,12 @@ CONFIG_MMC_SDHCI_S3C_DMA=y > CONFIG_MMC_DW=y > CONFIG_MMC_DW_IDMAC=y > CONFIG_MMC_DW_EXYNOS=y > +CONFIG_NEW_LEDS=y > +CONFIG_LEDS_CLASS=m > +CONFIG_LEDS_PWM=m > +CONFIG_LEDS_TRIGGERS=y > +CONFIG_LEDS_TRIGGER_TIMER=m > +CONFIG_LEDS_TRIGGER_HEARTBEAT=m > CONFIG_RTC_CLASS=y > CONFIG_RTC_DRV_MAX77686=y > CONFIG_RTC_DRV_MAX77802=y Reviewed-by: Lukasz Majewski -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
W dniu 08.06.2015 o 19:20, Anand Moon pisze: > On 8 June 2015 at 10:58, Vivek Gautam wrote: >> Hi, >> (...) >> >> On Monday, June 08, 2015 10:44 AM, "Krzysztof Kozlowski" >> wrote: >> >>> Untested code should not go to the kernel. Additionally you should >>> mark it as not-tested. Marking such patch as non-tested could help you >>> finding some independent tests (tests performed by someone else). >>> >>> To summarize my point of view: >>> 1. Unless Vivek's says otherwise, please give him the credits with >>> proper "from" field. >>> 2. Issues mentioned in previous mail should be addressed (missing >>> IS_ERR(), how disabling the regulator during suspend affects waking >>> up). >>> 3. The patchset must be tested, even after rebasing. >> >> >> Unfortunately, I got busy with a different project and lost track of the >> patches posted upstream. >> If it's not too late I can post a rebased version of the patch with previous >> review comments addressed. >> >>> >>> Best regards, >>> Krzysztof >> >> > > Hi All, > > I have learned my lesson not to interfere in others work. > It will never happen from my side again. > > Please accept my apology. Sure, no problem. Mistakes happen (I make a lot of them too), just learn from them. Javier explained what was the proper way to do this - retain original author of the patch. Your signed-off-by was correct. In fact the kernel SubmittingPatches is worth reading from time to time: https://www.kernel.org/doc/Documentation/SubmittingPatches Under chapter 11th ("Sign your work") in paragraph starting with "If you are a subsystem or branch maintainer" - it explains how to mention changes when modifying someone's else patch. You can also look at example from Bartlomiej, how hge did this when he took Thomas' patches: https://lkml.org/lkml/2015/4/3/387 However his case is kind of special because he made such a lot of changes and made such huge effort that in my humble opinion he could take the authorship of patch. But in this case the changelog is huge. And he tested it. Extensively. However he did not change the author of patches :) and you can just look at this work as an example. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/21] On-demand device registration
Am 04.06.2015 um 22:39 schrieb Alexander Holler: > As it seems to have been forgotten or overread, I've mentioned in my series of patches last year that, with a few changes, it's possible to let the algorithm I've used (dfs) to spit out all drivers which can be initialized in parallel. Unfortunately, I've missed that ... could you please resend you patches? Boot time reduction is one of the topics on my 2do in several weeks. cu -- Enrico Weigelt, metux IT consult +49-151-27565287 MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur für einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschließlich für denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, dürfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrtümlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte löschen Sie in diesem Fall diese Nachricht und alle Anhänge, ohne eine Kopie zu behalten. Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/11] dma: pl330: fix wording in mcbufsz message
On Wed, Jun 03, 2015 at 09:26:41PM +, Michal Suchanek wrote: > The kernel is not trying to increase mcbufsz. It suggests you should try > doing so. Also print the calculated required size of mcbufsz. pls use right subsystem name in the patches I have applied this now -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] usb: ehci-exynos: Make provision for vdd regulators
Facilitate getting required 3.3V and 1.0V VDD supply for EHCI controller on Exynos. For example, patches for regulators' nodes: c8c253f ARM: dts: Add regulator entries to smdk5420 275dcd2 ARM: dts: add max77686 pmic node for smdk5250, enable only minimum number of regulators on smdk5250. So ensuring now that the controller driver requests the necessary VDD regulators (if available, unless there are direct VDD rails), and enable them so as to make them working on exynos systems. Signed-off-by: Vivek Gautam Cc: Jingoo Han Cc: Krzysztof Kozlowski Cc: Alan Stern --- These patches had long been posted, and i lost track of them. My apologies for that. Thanks to Krzysztof for catching this. Kindly review. Changes since v3: - added a if (!IS_ERR()) check for regulators before disabling them in error path. Changes since v2: - replaced devm_regulator_get() with devm_regulator_get_optional(). - Added Documentation for the vdd supplies for the controller. - Re-did the commit message. .../devicetree/bindings/usb/exynos-usb.txt | 2 + drivers/usb/host/ehci-exynos.c | 57 +- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt index 9b4dbe3..776fa03 100644 --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt @@ -23,6 +23,8 @@ Required properties: Optional properties: - samsung,vbus-gpio: if present, specifies the GPIO that needs to be pulled up for the bus to be powered. + - vdd33-supply: handle to 3.3V Vdd supply regulator for the controller. + - vdd10-supply: handle to 1.0V Vdd supply regulator for the controller. Example: diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index df538fd..160ad66 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -45,6 +46,8 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver; struct exynos_ehci_hcd { struct clk *clk; struct phy *phy[PHY_NUMBER]; + struct regulator *vdd33; + struct regulator *vdd10; }; #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv) @@ -170,7 +173,27 @@ static int exynos_ehci_probe(struct platform_device *pdev) err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci); if (err) - goto fail_clk; + goto fail_regulator1; + + exynos_ehci->vdd33 = devm_regulator_get_optional(&pdev->dev, "vdd33"); + if (!IS_ERR(exynos_ehci->vdd33)) { + err = regulator_enable(exynos_ehci->vdd33); + if (err) { + dev_err(&pdev->dev, + "Failed to enable 3.3V Vdd supply\n"); + goto fail_regulator1; + } + } + + exynos_ehci->vdd10 = devm_regulator_get_optional(&pdev->dev, "vdd10"); + if (!IS_ERR(exynos_ehci->vdd10)) { + err = regulator_enable(exynos_ehci->vdd10); + if (err) { + dev_err(&pdev->dev, + "Failed to enable 1.0V Vdd supply\n"); + goto fail_regulator2; + } + } skip_phy: @@ -231,6 +254,12 @@ fail_add_hcd: fail_io: clk_disable_unprepare(exynos_ehci->clk); fail_clk: + if (!IS_ERR(exynos_ehci->vdd10)) + regulator_disable(exynos_ehci->vdd10); +fail_regulator2: + if (!IS_ERR(exynos_ehci->vdd33)) + regulator_disable(exynos_ehci->vdd33); +fail_regulator1: usb_put_hcd(hcd); return err; } @@ -246,6 +275,11 @@ static int exynos_ehci_remove(struct platform_device *pdev) clk_disable_unprepare(exynos_ehci->clk); + if (!IS_ERR(exynos_ehci->vdd33)) + regulator_disable(exynos_ehci->vdd33); + if (!IS_ERR(exynos_ehci->vdd10)) + regulator_disable(exynos_ehci->vdd10); + usb_put_hcd(hcd); return 0; @@ -268,6 +302,11 @@ static int exynos_ehci_suspend(struct device *dev) clk_disable_unprepare(exynos_ehci->clk); + if (!IS_ERR(exynos_ehci->vdd33)) + regulator_disable(exynos_ehci->vdd33); + if (!IS_ERR(exynos_ehci->vdd10)) + regulator_disable(exynos_ehci->vdd10); + return rc; } @@ -277,6 +316,22 @@ static int exynos_ehci_resume(struct device *dev) struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd); int ret; + if (!IS_ERR(exynos_ehci->vdd33)) { + ret = regulator_enable(exynos_ehci->vdd33); + if (ret) { + dev_err(dev, "Failed to enable 3.3V Vdd supply\n"); + return ret; + } + } + + if (!IS_ERR(exynos_ehci->vdd10))
[PATCH v4 2/2] usb: ohci-exynos: Make provision for vdd regulators
Facilitate getting required 3.3V and 1.0V VDD supply for OHCI controller on Exynos. For example, patches for regulators' nodes: c8c253f ARM: dts: Add regulator entries to smdk5420 275dcd2 ARM: dts: add max77686 pmic node for smdk5250, enable only minimum number of regulators on smdk5250. So ensuring now that the controller driver requests the necessary VDD regulators (if available, unless there are direct VDD rails), and enable them so as to make them working on exynos systems. Signed-off-by: Vivek Gautam Cc: Jingoo Han Cc: Krzysztof Kozlowski Cc: Alan Stern --- Changes since v3: - added a if (!IS_ERR()) check for regulators before disabling them in error path. Changes since v2: - replaced devm_regulator_get() with devm_regulator_get_optional(). - Added Documentation for the vdd supplies for the controller. - Re-did the commit message. .../devicetree/bindings/usb/exynos-usb.txt | 2 + .../devicetree/bindings/usb/exynos-usb.txt | 4 ++ drivers/usb/host/ohci-exynos.c | 57 +- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt index 776fa03..3883baa 100644 --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt @@ -63,6 +63,10 @@ Required properties: port 2 is HSIC phy1 - phys: from the *Generic PHY* bindings, specifying phy used by port. +Optional properties: + - vdd33-supply: handle to 3.3V Vdd supply regulator for the controller. + - vdd10-supply: handle to 1.0V Vdd supply regulator for the controller. + Example: usb@1212 { compatible = "samsung,exynos4210-ohci"; diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 2cd105b..4d6dea4 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,8 @@ static struct hc_driver __read_mostly exynos_ohci_hc_driver; struct exynos_ohci_hcd { struct clk *clk; struct phy *phy[PHY_NUMBER]; + struct regulator *vdd33; + struct regulator *vdd10; }; static int exynos_ohci_get_phy(struct device *dev, @@ -139,7 +142,27 @@ static int exynos_ohci_probe(struct platform_device *pdev) err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci); if (err) - goto fail_clk; + goto fail_regulator1; + + exynos_ohci->vdd33 = devm_regulator_get_optional(&pdev->dev, "vdd33"); + if (!IS_ERR(exynos_ohci->vdd33)) { + err = regulator_enable(exynos_ohci->vdd33); + if (err) { + dev_err(&pdev->dev, + "Failed to enable 3.3V Vdd supply\n"); + goto fail_regulator1; + } + } + + exynos_ohci->vdd10 = devm_regulator_get_optional(&pdev->dev, "vdd10"); + if (!IS_ERR(exynos_ohci->vdd10)) { + err = regulator_enable(exynos_ohci->vdd10); + if (err) { + dev_err(&pdev->dev, + "Failed to enable 1.0V Vdd supply\n"); + goto fail_regulator2; + } + } skip_phy: exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost"); @@ -191,6 +214,12 @@ fail_add_hcd: fail_io: clk_disable_unprepare(exynos_ohci->clk); fail_clk: + if (!IS_ERR(exynos_ohci->vdd10)) + regulator_disable(exynos_ohci->vdd10); +fail_regulator2: + if (!IS_ERR(exynos_ohci->vdd33)) + regulator_disable(exynos_ohci->vdd33); +fail_regulator1: usb_put_hcd(hcd); return err; } @@ -206,6 +235,11 @@ static int exynos_ohci_remove(struct platform_device *pdev) clk_disable_unprepare(exynos_ohci->clk); + if (!IS_ERR(exynos_ohci->vdd33)) + regulator_disable(exynos_ohci->vdd33); + if (!IS_ERR(exynos_ohci->vdd10)) + regulator_disable(exynos_ohci->vdd10); + usb_put_hcd(hcd); return 0; @@ -234,6 +268,11 @@ static int exynos_ohci_suspend(struct device *dev) clk_disable_unprepare(exynos_ohci->clk); + if (!IS_ERR(exynos_ohci->vdd33)) + regulator_disable(exynos_ohci->vdd33); + if (!IS_ERR(exynos_ohci->vdd10)) + regulator_disable(exynos_ohci->vdd10); + return 0; } @@ -243,6 +282,22 @@ static int exynos_ohci_resume(struct device *dev) struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd); int ret; + if (!IS_ERR(exynos_ohci->vdd33)) { + ret = regulator_enable(exynos_ohci->vdd33); + if (ret) { + dev_err(dev, "Failed to enable 3.3V Vdd supply\n"); + return ret; + } +
Re: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
Hi, On Monday, June 08, 2015 3:50 PM, "Anand Moon" On 8 June 2015 at 10:58, Vivek Gautam wrote: Hi, On Monday, June 08, 2015 10:44 AM, "Krzysztof Kozlowski" wrote: my apologies for being late in replying to this thread. 2015-06-08 13:21 GMT+09:00 Anand Moon : Hi Krzysztof , On 8 June 2015 at 07:40, Krzysztof Kozlowski wrote: On 07.06.2015 22:20, Anand Moon wrote: Facilitate getting required 3.3V and 1.0V VDD supply for EHCI controller on Exynos. With the patches for regulators' nodes merged in 3.15: c8c253f ARM: dts: Add regulator entries to smdk5420 275dcd2 ARM: dts: add max77686 pmic node for smdk5250, the exynos systems turn on only minimal number of regulators. Until now, the VDD regulator supplies were either turned on by the bootloader, or the regulators were enabled by default in the kernel, so that the controller drivers did not need to care about turning on these regulators on their own. This was rather bad about these controller drivers. So ensuring now that the controller driver requests the necessary VDD regulators (if available, unless there are direct VDD rails), and enable them so as to make them working. Signed-off-by: Vivek Gautam Signed-off-by: Anand Moon Cc: Jingoo Han Cc: Alan Stern --- Initial version of this patch was part of following series, though they are not dependent on each other, resubmitting after rebasing. http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266418.html So you just took Vivek's patch along with all the credits... That is not how we usually do this. I would expect that rebasing a patch won't change the author unless this is fine with Vivek. Sorry If I have done some mistake on my part. I just looked at below mail chain. Before I send it. http://www.spinics.net/lists/linux-samsung-soc/msg44136.html I don't get it. The patch you are referring to has a proper "From" field. So please use it as an example. I don't want to take any credit out of it. I just re-base on the new kernel. Perhaps, you would have maintained the authorship ! I could not test this patch as it meant for exynos5440 boards. Are you sure? I think the driver is used on almost all of Exynos SoCs (Exynos4, Exynos5250, Exynos542x). That's correct, as pointed by Krzysztof Kozlowski, the driver is same for Exynos4 and Exynos5 series of SoCs. Untested code should not go to the kernel. Additionally you should mark it as not-tested. Marking such patch as non-tested could help you finding some independent tests (tests performed by someone else). To summarize my point of view: 1. Unless Vivek's says otherwise, please give him the credits with proper "from" field. 2. Issues mentioned in previous mail should be addressed (missing IS_ERR(), how disabling the regulator during suspend affects waking up). 3. The patchset must be tested, even after rebasing. Unfortunately, I got busy with a different project and lost track of the patches posted upstream. If it's not too late I can post a rebased version of the patch with previous review comments addressed. Best regards, Krzysztof Hi All, I have learned my lesson not to interfere in others work. It will never happen from my side again. Please accept my apology. Please don't apologise. It's just a part of learning; learning how linux mainlining works. Hope you understand. :-) thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
On 8 June 2015 at 10:58, Vivek Gautam wrote: > Hi, > > > > On Monday, June 08, 2015 10:44 AM, "Krzysztof Kozlowski" > wrote: > > my apologies for being late in replying to this thread. > >> 2015-06-08 13:21 GMT+09:00 Anand Moon : >>> >>> Hi Krzysztof , >>> >>> On 8 June 2015 at 07:40, Krzysztof Kozlowski >>> wrote: On 07.06.2015 22:20, Anand Moon wrote: > > Facilitate getting required 3.3V and 1.0V VDD supply for > EHCI controller on Exynos. > > With the patches for regulators' nodes merged in 3.15: > c8c253f ARM: dts: Add regulator entries to smdk5420 > 275dcd2 ARM: dts: add max77686 pmic node for smdk5250, > the exynos systems turn on only minimal number of regulators. > > Until now, the VDD regulator supplies were either turned on > by the bootloader, or the regulators were enabled by default > in the kernel, so that the controller drivers did not need to > care about turning on these regulators on their own. > This was rather bad about these controller drivers. > So ensuring now that the controller driver requests the necessary > VDD regulators (if available, unless there are direct VDD rails), > and enable them so as to make them working. > > Signed-off-by: Vivek Gautam > Signed-off-by: Anand Moon > Cc: Jingoo Han > Cc: Alan Stern > --- > Initial version of this patch was part of following series, though > they are not dependent on each other, resubmitting after rebasing. > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266418.html So you just took Vivek's patch along with all the credits... That is not how we usually do this. I would expect that rebasing a patch won't change the author unless this is fine with Vivek. >>> >>> Sorry If I have done some mistake on my part. >>> I just looked at below mail chain. Before I send it. >>> >>> http://www.spinics.net/lists/linux-samsung-soc/msg44136.html >> >> >> I don't get it. The patch you are referring to has a proper "From" >> field. So please use it as an example. >> >>> >>> I don't want to take any credit out of it. I just re-base on the new >>> kernel. > > Perhaps, you would have maintained the authorship ! > >>> I could not test this patch as it meant for exynos5440 boards. >> >> >> Are you sure? I think the driver is used on almost all of Exynos SoCs >> (Exynos4, Exynos5250, Exynos542x). > > > That's correct, as pointed by Krzysztof Kozlowski, the driver is same for > Exynos4 and Exynos5 series > of SoCs. > >> Untested code should not go to the kernel. Additionally you should >> mark it as not-tested. Marking such patch as non-tested could help you >> finding some independent tests (tests performed by someone else). >> >> To summarize my point of view: >> 1. Unless Vivek's says otherwise, please give him the credits with >> proper "from" field. >> 2. Issues mentioned in previous mail should be addressed (missing >> IS_ERR(), how disabling the regulator during suspend affects waking >> up). >> 3. The patchset must be tested, even after rebasing. > > > Unfortunately, I got busy with a different project and lost track of the > patches posted upstream. > If it's not too late I can post a rebased version of the patch with previous > review comments addressed. > >> >> Best regards, >> Krzysztof > > Hi All, I have learned my lesson not to interfere in others work. It will never happen from my side again. Please accept my apology. -Anand Moon -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] drm/exynos: fix broken component binding in case of multiple pipelines
In case there are multiple pipelines and deferred probe occurs, only components of the first pipeline were bound. As a result only one pipeline was available. The main cause of this issue was dynamic generation of component match table - every component driver during probe registered itself on helper list, if there was at least one pipeline present on this list component match table were created without deferred components. This patch removes this helper list, instead it creates match table from existing devices requiring exynos_drm KMS drivers. This way match table do not depend on probe/deferral order and contains all KMS components. As a side effect patch makes the code cleaner and significantly smaller. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/exynos/exynos7_drm_decon.c | 14 +- drivers/gpu/drm/exynos/exynos_dp_core.c| 13 +- drivers/gpu/drm/exynos/exynos_drm_dpi.c| 20 +- drivers/gpu/drm/exynos/exynos_drm_drv.c| 283 - drivers/gpu/drm/exynos/exynos_drm_dsi.c| 29 +-- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 28 +-- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 18 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 22 +-- drivers/gpu/drm/exynos/exynos_mixer.c | 14 +- 9 files changed, 99 insertions(+), 342 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c index d659ba2..22cb067 100644 --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c @@ -769,11 +769,6 @@ static int decon_probe(struct platform_device *pdev) if (!ctx) return -ENOMEM; - ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, - EXYNOS_DISPLAY_TYPE_LCD); - if (ret) - return ret; - ctx->dev = dev; ctx->suspended = true; @@ -783,10 +778,8 @@ static int decon_probe(struct platform_device *pdev) of_node_put(i80_if_timings); ctx->regs = of_iomap(dev->of_node, 0); - if (!ctx->regs) { - ret = -ENOMEM; - goto err_del_component; - } + if (!ctx->regs) + return -ENOMEM; ctx->pclk = devm_clk_get(dev, "pclk_decon0"); if (IS_ERR(ctx->pclk)) { @@ -856,8 +849,6 @@ err_disable_pm_runtime: err_iounmap: iounmap(ctx->regs); -err_del_component: - exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CRTC); return ret; } @@ -870,7 +861,6 @@ static int decon_remove(struct platform_device *pdev) iounmap(ctx->regs); component_del(&pdev->dev, &decon_component_ops); - exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC); return 0; } diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index c9995b1..bf1fce2 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1344,11 +1344,6 @@ static int exynos_dp_probe(struct platform_device *pdev) dp->display.ops = &exynos_dp_display_ops; platform_set_drvdata(pdev, dp); - ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR, - dp->display.type); - if (ret) - return ret; - panel_node = of_parse_phandle(dev->of_node, "panel", 0); if (panel_node) { dp->panel = of_drm_find_panel(panel_node); @@ -1369,18 +1364,12 @@ static int exynos_dp_probe(struct platform_device *pdev) return -EPROBE_DEFER; } - ret = component_add(&pdev->dev, &exynos_dp_ops); - if (ret) - exynos_drm_component_del(&pdev->dev, - EXYNOS_DEVICE_TYPE_CONNECTOR); - - return ret; + return component_add(&pdev->dev, &exynos_dp_ops); } static int exynos_dp_remove(struct platform_device *pdev) { component_del(&pdev->dev, &exynos_dp_ops); - exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR); return 0; } diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c index 6dc328e..7cb6595 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c @@ -313,33 +313,19 @@ struct exynos_drm_display *exynos_dpi_probe(struct device *dev) ctx->dev = dev; ctx->dpms_mode = DRM_MODE_DPMS_OFF; - ret = exynos_drm_component_add(dev, - EXYNOS_DEVICE_TYPE_CONNECTOR, - ctx->display.type); - if (ret) - return ERR_PTR(ret); - ret = exynos_dpi_parse_dt(ctx); if (ret < 0) { devm_kfree(dev, ctx); - goto err_del_component; + return NULL; } if (ctx->panel_node) { ctx->panel = of_drm_find
[PATCH 1/3] drm/exynos: consolidate driver/device initialization code
Code registering different drivers and simple platform devices was dispersed across multiple sub-modules. This patch moves it to one place. As a result initialization code is shorter and cleaner and should simplify further development. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 221 +++ drivers/gpu/drm/exynos/exynos_drm_drv.h | 17 --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 27 drivers/gpu/drm/exynos/exynos_drm_vidi.c | 35 - 4 files changed, 139 insertions(+), 161 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 08b9a8c..5c5a72a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -38,8 +38,6 @@ #define DRIVER_MAJOR 1 #define DRIVER_MINOR 0 -static struct platform_device *exynos_drm_pdev; - static DEFINE_MUTEX(drm_component_lock); static LIST_HEAD(drm_component_list); @@ -527,7 +525,41 @@ static const struct component_master_ops exynos_drm_ops = { .unbind = exynos_drm_unbind, }; +static int exynos_drm_platform_probe(struct platform_device *pdev) +{ + struct component_match *match; + + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); + exynos_drm_driver.num_ioctls = ARRAY_SIZE(exynos_ioctls); + + match = exynos_drm_match_add(&pdev->dev); + if (IS_ERR(match)) { + return PTR_ERR(match); + } + + return component_master_add_with_match(&pdev->dev, &exynos_drm_ops, + match); +} + +static int exynos_drm_platform_remove(struct platform_device *pdev) +{ + component_master_del(&pdev->dev, &exynos_drm_ops); + return 0; +} + +static struct platform_driver exynos_drm_platform_driver = { + .probe = exynos_drm_platform_probe, + .remove = exynos_drm_platform_remove, + .driver = { + .name = "exynos-drm", + .pm = &exynos_drm_pm_ops, + }, +}; + static struct platform_driver *const exynos_drm_kms_drivers[] = { +#ifdef CONFIG_DRM_EXYNOS_VIDI + &vidi_driver, +#endif #ifdef CONFIG_DRM_EXYNOS_FIMD &fimd_driver, #endif @@ -562,30 +594,109 @@ static struct platform_driver *const exynos_drm_non_kms_drivers[] = { #ifdef CONFIG_DRM_EXYNOS_IPP &ipp_driver, #endif + &exynos_drm_platform_driver, }; -static int exynos_drm_platform_probe(struct platform_device *pdev) + +static struct platform_driver *const exynos_drm_drv_with_simple_dev[] = { +#ifdef CONFIG_DRM_EXYNOS_VIDI + &vidi_driver, +#endif +#ifdef CONFIG_DRM_EXYNOS_IPP + &ipp_driver, +#endif + &exynos_drm_platform_driver, +}; + +#define PDEV_COUNT ARRAY_SIZE(exynos_drm_drv_with_simple_dev) + +static struct platform_device *exynos_drm_pdevs[PDEV_COUNT]; + +static void exynos_drm_unregister_devices(void) { - struct component_match *match; + int i = PDEV_COUNT; - pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); - exynos_drm_driver.num_ioctls = ARRAY_SIZE(exynos_ioctls); + while (--i >= 0) { + platform_device_unregister(exynos_drm_pdevs[i]); + exynos_drm_pdevs[i] = NULL; + } +} - match = exynos_drm_match_add(&pdev->dev); - if (IS_ERR(match)) { - return PTR_ERR(match); +static int exynos_drm_register_devices(void) +{ + int i; + + for (i = 0; i < PDEV_COUNT; ++i) { + struct platform_driver *d = exynos_drm_drv_with_simple_dev[i]; + struct platform_device *pdev = + platform_device_register_simple(d->driver.name,-1, NULL, + 0); + + if (!IS_ERR(pdev)) { + exynos_drm_pdevs[i] = pdev; + continue; + } + while (--i >= 0) { + platform_device_unregister(exynos_drm_pdevs[i]); + exynos_drm_pdevs[i] = NULL; + } + + return PTR_ERR(pdev); } - return component_master_add_with_match(&pdev->dev, &exynos_drm_ops, - match); + return 0; } -static int exynos_drm_platform_remove(struct platform_device *pdev) +static void exynos_drm_unregister_drivers(struct platform_driver * const *drv, + int count) { - component_master_del(&pdev->dev, &exynos_drm_ops); + while (--count >= 0) + platform_driver_unregister(drv[count]); +} + +static int exynos_drm_register_drivers(struct platform_driver * const *drv, + int count) +{ + int i, ret; + + for (i = 0; i < count; ++i) { + ret = platform_driver_register(drv[i]); + if (!ret) + continue; + + while (--i >= 0) +
[PATCH 3/3] drm/exynos: remove SoC checking code
SoC checking code is not necessary anymore, as exynos_drm_match_add and exynos_drm_platform_probe already properly handles situation when there are no Exynos DRM components. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 27 +-- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index cfbfb6c..9ec4027 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -560,34 +560,9 @@ static inline void exynos_drm_unregister_non_kms_drivers(void) ARRAY_SIZE(exynos_drm_non_kms_drivers)); } -static const char * const strings[] = { - "samsung,exynos3", - "samsung,exynos4", - "samsung,exynos5", - "samsung,exynos7", -}; - static int exynos_drm_init(void) { - bool is_exynos = false; - int ret, i; - - /* -* Register device object only in case of Exynos SoC. -* -* Below codes resolves temporarily infinite loop issue incurred -* by Exynos drm driver when using multi-platform kernel. -* So these codes will be replaced with more generic way later. -*/ - for (i = 0; i < ARRAY_SIZE(strings); i++) { - if (of_machine_is_compatible(strings[i])) { - is_exynos = true; - break; - } - } - - if (!is_exynos) - return -ENODEV; + int ret; ret = exynos_drm_register_devices(); if (ret) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] exynos_drm initialization fix
Hi Inki, Those patches fixes issue of exynos_drm initialization with more than one pipeline, which is present in exynos_drm drivers for long time. On most platforms at least two of three following pipelines are present: 1. VIDI. 2. FIMD/DECON -> DSI/DPI -> Panel. 3. MIXER/DECON -> HDMI -> TV. In case we want use two or three pipelines simultaneously and one of KMS drivers defers probing only some drivers will be bound to drm driver and usually only one pipeline will be available. This patchset fixes it. Beside bugfixing the patchset significantly simplifies and clears the code - about 300 lines removed. The patchset is based on exynos-drm-next branch. The patchset was tested on universal_c210 board. Regards Andrzej Andrzej Hajda (3): drm/exynos: consolidate driver/device initialization code drm/exynos: fix broken component binding in case of multiple pipelines drm/exynos: remove SoC checking code drivers/gpu/drm/exynos/exynos7_drm_decon.c | 14 +- drivers/gpu/drm/exynos/exynos_dp_core.c| 13 +- drivers/gpu/drm/exynos/exynos_drm_dpi.c| 20 +- drivers/gpu/drm/exynos/exynos_drm_drv.c| 427 +++-- drivers/gpu/drm/exynos/exynos_drm_drv.h| 17 -- drivers/gpu/drm/exynos/exynos_drm_dsi.c| 29 +- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 28 +- drivers/gpu/drm/exynos/exynos_drm_ipp.c| 27 -- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 53 +--- drivers/gpu/drm/exynos/exynos_hdmi.c | 22 +- drivers/gpu/drm/exynos/exynos_mixer.c | 14 +- 11 files changed, 187 insertions(+), 477 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
-- From: "Krzysztof Kozlowski" Sent: Monday, June 08, 2015 7:40 AM To: "Anand Moon" ; "Rob Herring" ; "Pawel Moll" ; "Mark Rutland" ; "Ian Campbell" ; "Kumar Gala" ; "Kukjin Kim" ; "Alan Stern" ; "Greg Kroah-Hartman" ; "Vivek Gautam" ; "Felipe Balbi" Cc: ; ; ; ; ; "Jingoo Han" Subject: Re: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators On 07.06.2015 22:20, Anand Moon wrote: Facilitate getting required 3.3V and 1.0V VDD supply for EHCI controller on Exynos. With the patches for regulators' nodes merged in 3.15: c8c253f ARM: dts: Add regulator entries to smdk5420 275dcd2 ARM: dts: add max77686 pmic node for smdk5250, the exynos systems turn on only minimal number of regulators. Until now, the VDD regulator supplies were either turned on by the bootloader, or the regulators were enabled by default in the kernel, so that the controller drivers did not need to care about turning on these regulators on their own. This was rather bad about these controller drivers. So ensuring now that the controller driver requests the necessary VDD regulators (if available, unless there are direct VDD rails), and enable them so as to make them working. Signed-off-by: Vivek Gautam Signed-off-by: Anand Moon Cc: Jingoo Han Cc: Alan Stern --- Initial version of this patch was part of following series, though they are not dependent on each other, resubmitting after rebasing. http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266418.html So you just took Vivek's patch along with all the credits... That is not how we usually do this. I would expect that rebasing a patch won't change the author unless this is fine with Vivek. --- .../devicetree/bindings/usb/exynos-usb.txt | 2 + drivers/usb/host/ehci-exynos.c | 55 +- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt index 9b4dbe3..776fa03 100644 --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt @@ -23,6 +23,8 @@ Required properties: Optional properties: - samsung,vbus-gpio: if present, specifies the GPIO that needs to be pulled up for the bus to be powered. + - vdd33-supply: handle to 3.3V Vdd supply regulator for the controller. + - vdd10-supply: handle to 1.0V Vdd supply regulator for the controller. Example: diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index df538fd..4f8f9d2 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -45,6 +46,8 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver; struct exynos_ehci_hcd { struct clk *clk; struct phy *phy[PHY_NUMBER]; + struct regulator *vdd33; + struct regulator *vdd10; }; #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv) @@ -170,7 +173,27 @@ static int exynos_ehci_probe(struct platform_device *pdev) err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci); if (err) - goto fail_clk; + goto fail_regulator1; + + exynos_ehci->vdd33 = devm_regulator_get_optional(&pdev->dev, "vdd33"); + if (!IS_ERR(exynos_ehci->vdd33)) { + err = regulator_enable(exynos_ehci->vdd33); + if (err) { + dev_err(&pdev->dev, + "Failed to enable 3.3V Vdd supply\n"); + goto fail_regulator1; + } + } + + exynos_ehci->vdd10 = devm_regulator_get_optional(&pdev->dev, "vdd10"); + if (!IS_ERR(exynos_ehci->vdd10)) { + err = regulator_enable(exynos_ehci->vdd10); + if (err) { + dev_err(&pdev->dev, + "Failed to enable 1.0V Vdd supply\n"); + goto fail_regulator2; + } + } skip_phy: @@ -231,6 +254,10 @@ fail_add_hcd: fail_io: clk_disable_unprepare(exynos_ehci->clk); fail_clk: + regulator_disable(exynos_ehci->vdd10); +fail_regulator2: + regulator_disable(exynos_ehci->vdd33); if (!IS_ERR()). +fail_regulator1: usb_put_hcd(hcd); return err; } @@ -246,6 +273,11 @@ static int exynos_ehci_remove(struct platform_device *pdev) clk_disable_unprepare(exynos_ehci->clk); + if (!IS_ERR(exynos_ehci->vdd33)) + regulator_disable(exynos_ehci->vdd33); + if (!IS_ERR(exynos_ehci->vdd10)) + regulator_disable(exynos_ehci->vdd10); + usb_put_hcd(hcd); return 0; @@ -268,6 +300,11 @@ static int exynos_ehci_suspend(struct device *dev) clk_disable_unprepare(exynos_ehci->clk); + if (!IS_ERR(exynos_ehci->vdd33)) + regulator_disable(exynos_ehci->vdd33); + if (!IS_ERR(exynos_ehci->vdd10)) + regulator_disable(exynos_ehci->vdd10); + Is EHCI a wakeup source? If yes then how disabling regulators during suspend affects waking up process? From my knowledge of Exynos5 USB controller, EHCI could not be used as the wake up source for suspend. That's the reason we tried this approach of gating the regulator supplies to the controller during
Re: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
Hi, On Monday, June 08, 2015 10:44 AM, "Krzysztof Kozlowski" wrote: my apologies for being late in replying to this thread. 2015-06-08 13:21 GMT+09:00 Anand Moon : Hi Krzysztof , On 8 June 2015 at 07:40, Krzysztof Kozlowski wrote: On 07.06.2015 22:20, Anand Moon wrote: Facilitate getting required 3.3V and 1.0V VDD supply for EHCI controller on Exynos. With the patches for regulators' nodes merged in 3.15: c8c253f ARM: dts: Add regulator entries to smdk5420 275dcd2 ARM: dts: add max77686 pmic node for smdk5250, the exynos systems turn on only minimal number of regulators. Until now, the VDD regulator supplies were either turned on by the bootloader, or the regulators were enabled by default in the kernel, so that the controller drivers did not need to care about turning on these regulators on their own. This was rather bad about these controller drivers. So ensuring now that the controller driver requests the necessary VDD regulators (if available, unless there are direct VDD rails), and enable them so as to make them working. Signed-off-by: Vivek Gautam Signed-off-by: Anand Moon Cc: Jingoo Han Cc: Alan Stern --- Initial version of this patch was part of following series, though they are not dependent on each other, resubmitting after rebasing. http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266418.html So you just took Vivek's patch along with all the credits... That is not how we usually do this. I would expect that rebasing a patch won't change the author unless this is fine with Vivek. Sorry If I have done some mistake on my part. I just looked at below mail chain. Before I send it. http://www.spinics.net/lists/linux-samsung-soc/msg44136.html I don't get it. The patch you are referring to has a proper "From" field. So please use it as an example. I don't want to take any credit out of it. I just re-base on the new kernel. Perhaps, you would have maintained the authorship ! I could not test this patch as it meant for exynos5440 boards. Are you sure? I think the driver is used on almost all of Exynos SoCs (Exynos4, Exynos5250, Exynos542x). That's correct, as pointed by Krzysztof Kozlowski, the driver is same for Exynos4 and Exynos5 series of SoCs. Untested code should not go to the kernel. Additionally you should mark it as not-tested. Marking such patch as non-tested could help you finding some independent tests (tests performed by someone else). To summarize my point of view: 1. Unless Vivek's says otherwise, please give him the credits with proper "from" field. 2. Issues mentioned in previous mail should be addressed (missing IS_ERR(), how disabling the regulator during suspend affects waking up). 3. The patchset must be tested, even after rebasing. Unfortunately, I got busy with a different project and lost track of the patches posted upstream. If it's not too late I can post a rebased version of the patch with previous review comments addressed. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] clk: change clk_ops' ->round_rate() prototype
Hi Boris, On 05/06/15 12:39, Boris Brezillon wrote: > Hi Jon, > > On Fri, 5 Jun 2015 09:46:09 +0100 > Jon Hunter wrote: > >> >> On 05/06/15 00:02, Paul Walmsley wrote: >>> Hi folks >>> >>> just a brief comment on this one: >>> >>> On Thu, 30 Apr 2015, Boris Brezillon wrote: >>> Clock rates are stored in an unsigned long field, but ->round_rate() (which returns a rounded rate from a requested one) returns a long value (errors are reported using negative error codes), which can lead to long overflow if the clock rate exceed 2Ghz. Change ->round_rate() prototype to return 0 or an error code, and pass the requested rate as a pointer so that it can be adjusted depending on hardware capabilities. >>> >>> ... >>> diff --git a/Documentation/clk.txt b/Documentation/clk.txt index 0e4f90a..fca8b7a 100644 --- a/Documentation/clk.txt +++ b/Documentation/clk.txt @@ -68,8 +68,8 @@ the operations defined in clk.h: int (*is_enabled)(struct clk_hw *hw); unsigned long (*recalc_rate)(struct clk_hw *hw, unsigned long parent_rate); - long(*round_rate)(struct clk_hw *hw, - unsigned long rate, + int (*round_rate)(struct clk_hw *hw, + unsigned long *rate, unsigned long *parent_rate); long(*determine_rate)(struct clk_hw *hw, unsigned long rate, >>> >>> I'd suggest that we should probably go straight to 64-bit rates. There >>> are already plenty of clock sources that can generate rates higher than >>> 4GiHz. >> >> An alternative would be to introduce to a frequency "base" the default >> could be Hz (for backwards compatibility), but for CPUs we probably only >> care about MHz (or may be kHz) and so 32-bits would still suffice. Even >> if CPUs cared about Hz they could still use Hz, but in that case they >> probably don't care about GHz. Obviously, we don't want to break DT >> compatibility but may be the frequency base could be defined in DT and >> if it is missing then Hz is assumed. Just a thought ... > > Yes, but is it really worth the additional complexity. You'll have to > add the unit information anyway, so using an unsigned long for the > value and another field for the unit (an enum ?) is just like using a > 64 bit integer. For a storage perspective, yes it would be the same. However, there are probably a lot of devices that would not need the extra range, but would now have to deal with 64-bit types. I have no idea how much overhead that would be in reality. If the overhead is negligible then a 64-bit type is definitely the way to go, as I agree it is simpler and cleaner. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cpufreq:exynos-cpufreq - Fix for memory leak in case SOC name does not match.
Hi Viresh, > On 25-05-15, 07:39, Shailendra Verma wrote: > > During probe free the memory allocated to "exynos_info" in case of > > unknown SOC type. > > > > Signed-off-by: Shailendra Verma > > --- > > drivers/cpufreq/exynos-cpufreq.c |6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cpufreq/exynos-cpufreq.c > > b/drivers/cpufreq/exynos-cpufreq.c index 82d2fbb..8682378 100644 > > --- a/drivers/cpufreq/exynos-cpufreq.c > > +++ b/drivers/cpufreq/exynos-cpufreq.c > > @@ -182,7 +182,7 @@ static int exynos_cpufreq_probe(struct > > platform_device *pdev) ret = exynos5250_cpufreq_init(exynos_info); > > } else { > > pr_err("%s: Unknown SoC type\n", __func__); > > - return -ENODEV; > > + ret = -ENODEV; > > } > > > > if (ret) > > @@ -190,12 +190,14 @@ static int exynos_cpufreq_probe(struct > > platform_device *pdev) > > if (exynos_info->set_freq == NULL) { > > dev_err(&pdev->dev, "No set_freq function > > (ERR)\n"); > > + ret = -EINVAL; > > goto err_vdd_arm; > > } > > > > arm_regulator = regulator_get(NULL, "vdd_arm"); > > if (IS_ERR(arm_regulator)) { > > dev_err(&pdev->dev, "failed to get resource > > vdd_arm\n"); > > + ret = -EINVAL; > > goto err_vdd_arm; > > } > > > > @@ -227,7 +229,7 @@ err_cpufreq_reg: > > regulator_put(arm_regulator); > > err_vdd_arm: > > kfree(exynos_info); > > - return -EINVAL; > > + return ret; > > } > > > > static struct platform_driver exynos_cpufreq_platdrv = { > > Acked-by: Viresh Kumar > Acked-by: Lukasz Majewski -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()
Hello, On 2015-06-04 15:13, Inki Dae wrote: On 2015년 06월 04일 22:08, Inki Dae wrote: On 2015년 06월 03일 17:26, Marek Szyprowski wrote: One should not do any assumptions on the stare of the fimd hardware during driver initialization, so to properly reset fimd before enabling IOMMU, one should ensure that all power domains and clocks are really enabled. This patch adds pm_runtime and clocks management in the fimd_clear_channel() function to ensure that any access to fimd registers will be performed with clocks and power domains enabled. I have tested this patch series on trats2 board which uses Exynos4412 SoC. However, the booting is halted out. Without iommu, the booting and display works well. For this test, I also merged another your patch series in iommu exynos tree and added device node relevant codes like below, in exynos4.dtsi file: fimd: fimd@11c0 { ... iommus = <&sysmmu_fimd0>; ... sysmmu_fimd0: sysmmu@11E2 { compatible = "samsung,exynos-sysmmu"; reg = <0x11E2 0x1000>; interrupt-parent = <&combiner>; interrupts = <5 2>; clock-names = "sysmmu", "master"; clocks = <&clock CLK_SMMU_FIMD0>, <&clock CLK_FIMD0>; power-domains = <&pd_lcd0>; #iommu-cells = <0>; }; in exynos4412-trats2.dts file: fimd@11c0 { status = "okay"; iommu-reserved-mapping = <0x4000 0x4000 0x4000>; }; Can you check it out? In addition, sometimes I see below kernel panic logs which means page fault to fimd occurs while booting: It looks that you didn't apply patch '[PATCH v7 24/25] ARM: DMA-mapping: add support for creating reserved mappings in iova space' (http://thread.gmane.org/gmane.linux.kernel.samsung-soc/45416/focus=45429 ). There was no consensus on it and it was left unmerged. I will check if it can be reworked on top of recently introduced iommu default domains feature, however it would be great if the fixed for FIMD and DRM gets merged earlier, so the issues in the drivers will no longer be a source of the problem. [0.394228] 1380.serial: ttySAC0 at MMIO 0x1380 (irq = 56, base_baud = 0) is a S3C6400/10 [0.394788] 1381.serial: ttySAC1 at MMIO 0x1381 (irq = 57, base_baud = 0) is a S3C6400/10 [0.395281] 1382.serial: ttySAC2 at MMIO 0x1382 (irq = 58, base_baud = 0) is a S3C6400/10 [1.122219] console [ttySAC2] enabled [1.126419] 1383.serial: ttySAC3 at MMIO 0x1383 (irq = 59, base_baud = 0) is a S3C6400/10 [1.136250] [drm] Initialized drm 1.1.0 20060810 [1.142710] PAGE FAULT occurred at 0x52188000 by 11e2.sysmmu(Page table base: 0x6ea8) [1.149754] Lv1 entry: 0x6e92dc01 [1.153172] [ cut here ] [1.157740] kernel BUG at drivers/iommu/exynos-iommu.c:364! [1.163296] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM [1.169110] Modules linked in: [1.172154] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-00563-gee14f4e-dirty #1384 [1.180394] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [1.186472] task: c06d2df0 ti: c06ce000 task.ti: c06ce000 [1.191861] PC is at exynos_sysmmu_irq+0x184/0x208 [1.196628] LR is at exynos_sysmmu_irq+0xd4/0x208 [1.201316] pc : []lr : []psr: 6193 [1.201316] sp : c06cfe90 ip : fp : [1.212772] r10: c06ff6a3 r9 : 0521 r8 : 52188000 [1.217980] r7 : eea8 r6 : ee9b3428 r5 : ee9b3410 r4 : [1.224489] r3 : 6e92dc01 r2 : 6e92dc01 r1 : eea55810 r0 : ee9c4e00 [1.231002] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel [1.238378] Control: 10c5387d Table: 4000404a DAC: 0015 [1.244107] Process swapper/0 (pid: 0, stack limit = 0xc06ce210) [1.250096] Stack: (0xc06cfe90 to 0xc06d) [1.254438] fe80: c06cfe9c c039caac eef82b80 6ea8 [1.262599] fea0: 16b1580f ee9b4240 ee84ad20 0026 ee84acc0 c0060308 [1.270757] fec0: eef81380 ee84acc0 ee84ad20 ee9b4240 0015 ee804450 c06cff68 [1.278916] fee0: ee808000 c0060400 ee84acc0 ee84ad20 ee807000 c00630a8 0026 c06dddb8 [1.287075] ff00: ee807000 c005f98c 000a c0200ccc 0015 0015 [1.295235] ff20: 0001 c005f98c c06caaac c005fc58 f002000c 0015 c06d07a0 c06cff68 [1.303394] ff40: f002 c06ff6a1 0001 c0009434 c0010068 6113 c06cff9c [1.311553] ff60: c06cffb8 c0012f40 1288 c001c880 c06ce000 c06d04f8 [1.319712] ff80: c04a1f50 c06c92c4 c06cffb8 c06ff6a1 0001 0100 c06cffb0 [1.327871] ffa0: c0010064 c0010068 6113 c06ce000 c0053f58 c067bc54 [1.336031] ffc0: c067b678 c06a70b8 c07023d4 c06d0480 [1.344190] ffe0: c06a70b4 c06d3f04 4000406a 413fc090 4000807c 0
Re: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
Hello Krzysztof, On Mon, Jun 8, 2015 at 8:52 AM, Krzysztof Kozlowski wrote: > 2015-06-08 15:42 GMT+09:00 Javier Martinez Canillas : >> Hello, >> >> On Mon, Jun 8, 2015 at 7:14 AM, Krzysztof Kozlowski >> wrote: >> >> [...] >> >>> >>> To summarize my point of view: >>> 1. Unless Vivek's says otherwise, please give him the credits with >>> proper "from" field. >>> 2. Issues mentioned in previous mail should be addressed (missing >>> IS_ERR(), how disabling the regulator during suspend affects waking >>> up). >>> 3. The patchset must be tested, even after rebasing. >>> >> >> Agreed with all the points. >> >> Anand, >> >> An easy way to preserve authorship when rebasing patches is to use the >> git command author option. As an example you can execute the following >> command: >> >> $ git commit -a -s --author='Vivek Gautam ' > > By default "git am" and "git format-patch" preserve the author of a > patch so usually this step is not necessary. Unless the patch is > applied in a different way... :) > That is correct but if an old patch still applies cleanly on top of current's tree, then there is no need to do a rebase right? ;-) I mean, git am is not as smart as the patch command for example to detect when the line numbers mentioned in the patch are incorrect and does not attempt to find the correct place to apply each hunk of the patch (at least by default, I don't know if there is an option). Which IIUC is what Anand had to do so in that case you need to manually commit again but using the original patch author. > Best regards, > Krzysztof Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html