Re: [PATCH V3 2/3] thermal: exynos: Miscellaneous fixes to support falling threshold interrupt
On 1/17/13, Amit Daniel Kachhap wrote: > Below fixes are done to support falling threshold interrupt, > * Falling interrupt status macro corrected according to exynos5 data sheet. > * The get trend function modified to calculate trip temperature correctly. > * The clearing of interrupt status in the isr is now done after handling > the event that caused the interrupt. One fix per patch would have been better. However not particular. Also are they applicable to older kernels? > > Signed-off-by: Amit Daniel Kachhap > --- -- 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 0/7] i2c: s3c2410: Add devm_* apis and cleanup
On Fri, Nov 23, 2012 at 11:29 AM, Tushar Behera wrote: > This patchset cleans up the probe function of i2c-s3c2410 driver. > These have been tested on Exynos4210 based Origen board. > > Tushar Behera (7): > i2c: s3c2410: Remove unnecessary label err_noclk > i2c: s3c2410: Convert to use devm_clk_get() > i2c: s3c2410: Convert to use devm_request_mem_region() > i2c: s3c2410: Convert to use devm_ioremap() > i2c: s3c2410: Convert to use devm_request_irq() You may want to consider request_and_ioremap. > i2c: s3c2410: Move location of clk_prepare_enable() call in probe > function > i2c: s3c2410: Remove err_cpufreq label > > drivers/i2c/busses/i2c-s3c2410.c | 74 > -- > 1 files changed, 23 insertions(+), 51 deletions(-) > > -- > 1.7.4.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 2/2] i2c-s3c2410: Convert to devm_request_and_ioremap()
On Tue, Nov 6, 2012 at 1:40 PM, Mark Brown wrote: > On Mon, Nov 05, 2012 at 05:21:03PM +0530, Shubhrajyoti Datta wrote: >> On Mon, Nov 5, 2012 at 2:03 PM, Mark Brown >> wrote: >> > A small code saving and less error handling to worry about. > >> Looks good. > >> request irq could be devm_* also. Not an objection though. > > devm_ is a much worse idea for IRQs than for other resource types since > interrupts are delivered asynchronously. Using it safely requires that > we do the analysis required to make sure that the hardware is totally > idle and can't interrupt. hmm thats true. thanks, -- 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 2/2] i2c-s3c2410: Convert to devm_request_and_ioremap()
On Mon, Nov 5, 2012 at 2:03 PM, Mark Brown wrote: > A small code saving and less error handling to worry about. > Looks good. request irq could be devm_* also. Not an objection though. -- 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] i2c-s3c2410: Use plain pm_runtime_put()
Hi Mark, On Thu, Jun 28, 2012 at 6:38 PM, Mark Brown wrote: > There's no point in using _sync() as we don't really care if the suspend > has completed immediately. Agree. Reviewed-by: Shubhrajyoti D > > Signed-off-by: Mark Brown > --- > drivers/i2c/busses/i2c-s3c2410.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index ddd5797..c4a4494 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -608,7 +608,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap, > > if (ret != -EAGAIN) { > clk_disable(i2c->clk); > - pm_runtime_put_sync(&adap->dev); > + pm_runtime_put(&adap->dev); Especially if we are retrying in 100us :-) > return ret; > } > > @@ -618,7 +618,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap, > } > > clk_disable(i2c->clk); > - pm_runtime_put_sync(&adap->dev); > + pm_runtime_put(&adap->dev); > return -EREMOTEIO; > } > > -- > 1.7.10 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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] serial: samsung: protect NULL dereference of clock name
Hi Kim, On Wed, May 30, 2012 at 1:59 PM, Kyoungil Kim wrote: > From: KeyYoung Park > > When priting the serial clock source, if clock source name is null, > kernel reference NULL point. > Could you help me understand why is that NULL ? Or the crash that you saw. -- 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 3/3] i2c-s3c2410: Refactor ifdefs for PM_SLEEP
On Tue, Feb 14, 2012 at 4:35 AM, Mark Brown wrote: > Use the PM_SLEEP ifdef for system suspend and resume. This is partly > in preparation for adding runtime operations and partly because a user > may in theory choose to enable runtime suspend but not system suspend. > Yes also rand defconfig may see some warns. Looks good to me Reviewed-by: Shubhrajyoti D > Signed-off-by: Mark Brown > --- > drivers/i2c/busses/i2c-s3c2410.c | 6 +- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index 1cb06c58..5d8e802 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -1073,7 +1073,7 @@ static int s3c24xx_i2c_remove(struct platform_device > *pdev) > return 0; > } > > -#ifdef CONFIG_PM > +#ifdef CONFIG_PM_SLEEP > static int s3c24xx_i2c_suspend_noirq(struct device *dev) > { > struct platform_device *pdev = to_platform_device(dev); > @@ -1096,10 +1096,14 @@ static int s3c24xx_i2c_resume(struct device *dev) > > return 0; > } > +#endif > > +#ifdef CONFIG_PM > static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = { > +#ifdef CONFIG_PM_SLEEP > .suspend_noirq = s3c24xx_i2c_suspend_noirq, > .resume = s3c24xx_i2c_resume, > +#endif > }; > > #define S3C24XX_DEV_PM_OPS (&s3c24xx_i2c_dev_pm_ops) > -- > 1.7.9.rc1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 1/2] i2c-s3c2410: Convert to devm_kzalloc()
Hi Mark, The changes look good to me. Reviewed-by: Datta Shubhrajyoti For your changes. Some other doubts though not related to your patch. Just curious. On Sat, Jan 21, 2012 at 6:58 PM, Mark Brown wrote: > Saves remembering to call kfree(). There's some kfree()s used by the > resource still, these will be removed in 3.3 using the newly added > devm_request_and_ioremap(). > > Signed-off-by: Mark Brown > --- > drivers/i2c/busses/i2c-s3c2410.c | 4 +--- > 1 files changed, 1 insertions(+), 3 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c > b/drivers/i2c/busses/i2c-s3c2410.c > index 4c17180..e6f982b 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -890,7 +890,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) > } > } > > - i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL); > + i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), > GFP_KERNEL); > if (!i2c) { > dev_err(&pdev->dev, "no memory for state\n"); The comment looks confusing as I am not sure what is meant here by state. > return -ENOMEM; > @@ -1035,7 +1035,6 @@ static int s3c24xx_i2c_probe(struct platform_device > *pdev) > clk_put(i2c->clk); > > err_noclk: > - kfree(i2c); > return ret; > } > > @@ -1061,7 +1060,6 @@ static int s3c24xx_i2c_remove(struct platform_device > *pdev) > release_resource(i2c->ioarea); > s3c24xx_i2c_dt_gpio_free(i2c); > kfree(i2c->ioarea); Do we need both release_resource and kfree > - kfree(i2c); > > return 0; > } > -- > 1.7.7.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 1/3] spi/s3c64xx: Log error interrupts
Hi Mark, On Mon, Jan 30, 2012 at 3:10 AM, Mark Brown wrote: > On Sat, Jan 28, 2012 at 10:30:36PM +0530, Shubhrajyoti Datta wrote: > > Please delete irrelevant context from your replies. > >> > + ret = request_irq(irq, s3c64xx_spi_irq, 0, "spi-s3c64xx", sdd); > >> Could we have a threaded irq instead and there are prints and that may get >> executed in interrupt context. > > So, clearly we *could*. However I think that's not going to be as > helpful. These errors are never supposed to happen (and if they do > indicate a fairly serious problem) so the performance impact shouldn't > be our primary concern and if they do happen then my suspicion is that > the timing information will be very important. I expect that knowing > exactly when the error occurred is likely to help diagnose what caused > it by virtue of knowing what other things were going on in the system at > that time. I agree . My only concern was that incase someone wants to add prints in the irq that may be handy. However as you rightly pointed that timing is surely to be more of a concern. Thanks for the explanation. -- 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 1/3] spi/s3c64xx: Log error interrupts
Hi Mark, Some minor doubts/ questions. On 1/21/12, Mark Brown wrote: > Although the hardware supports interrupts we're not currently using them > at all since for small transfers the overhead is greater than that for > busy waiting and for large transfers we have interrupts from the DMA. > This means that if the hardware reports an error (especially one which > might not stall transfer) we might miss it. > > Take a first pass at dealing with such errors by enabling the interrupt > if we can and logging the errors if they happen. Ideally we'd report the > error via the affected transfer but since we're in master mode it's very > difficult to trigger errors at present and this code is much simpler. > > Signed-off-by: Mark Brown > Acked-by: Linus Walleij > --- > drivers/spi/spi-s3c64xx.c | 57 > +++- > 1 files changed, 55 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 019a716..d56066b 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -153,6 +154,7 @@ struct s3c64xx_spi_dma_data { > * @tx_dmach: Controller's DMA channel for Tx. > * @sfr_start: BUS address of SPI controller regs. > * @regs: Pointer to ioremap'ed controller registers. > + * @irq: interrupt > * @xfer_completion: To indicate completion of xfer task. > * @cur_mode: Stores the active configuration of the controller. > * @cur_bpw: Stores the active bits per word settings. > @@ -930,6 +932,33 @@ setup_exit: > return err; > } > > +static irqreturn_t s3c64xx_spi_irq(int irq, void *data) > +{ > + struct s3c64xx_spi_driver_data *sdd = data; > + struct spi_master *spi = sdd->master; > + unsigned int val; > + > + val = readl(sdd->regs + S3C64XX_SPI_PENDING_CLR); > + > + val &= S3C64XX_SPI_PND_RX_OVERRUN_CLR | > + S3C64XX_SPI_PND_RX_UNDERRUN_CLR | > + S3C64XX_SPI_PND_TX_OVERRUN_CLR | > + S3C64XX_SPI_PND_TX_UNDERRUN_CLR; > + > + writel(val, sdd->regs + S3C64XX_SPI_PENDING_CLR); > + > + if (val & S3C64XX_SPI_PND_RX_OVERRUN_CLR) > + dev_err(&spi->dev, "RX overrun\n"); > + if (val & S3C64XX_SPI_PND_RX_UNDERRUN_CLR) > + dev_err(&spi->dev, "RX underrun\n"); > + if (val & S3C64XX_SPI_PND_TX_OVERRUN_CLR) > + dev_err(&spi->dev, "TX overrun\n"); > + if (val & S3C64XX_SPI_PND_TX_UNDERRUN_CLR) > + dev_err(&spi->dev, "TX underrun\n"); > + > + return IRQ_HANDLED; > +} > + > static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int > channel) > { > struct s3c64xx_spi_info *sci = sdd->cntrlr_info; > @@ -970,7 +999,8 @@ static int __init s3c64xx_spi_probe(struct > platform_device *pdev) > struct s3c64xx_spi_driver_data *sdd; > struct s3c64xx_spi_info *sci; > struct spi_master *master; > - int ret; > + int ret, irq; > + char clk_name[16]; > > if (pdev->id < 0) { > dev_err(&pdev->dev, > @@ -1010,6 +1040,12 @@ static int __init s3c64xx_spi_probe(struct > platform_device *pdev) > return -ENXIO; > } > > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_warn(&pdev->dev, "Failed to get IRQ: %d\n", irq); > + return irq; > + } > + > master = spi_alloc_master(&pdev->dev, > sizeof(struct s3c64xx_spi_driver_data)); > if (master == NULL) { > @@ -1104,10 +1140,21 @@ static int __init s3c64xx_spi_probe(struct > platform_device *pdev) > INIT_WORK(&sdd->work, s3c64xx_spi_work); > INIT_LIST_HEAD(&sdd->queue); > > + ret = request_irq(irq, s3c64xx_spi_irq, 0, "spi-s3c64xx", sdd); Could we have a threaded irq instead and there are prints and that may get executed in interrupt context. > + if (ret != 0) { > + dev_err(&pdev->dev, "Failed to request IRQ %d: %d\n", > + irq, ret); > + goto err8; > + } > + > + writel(S3C64XX_SPI_INT_RX_OVERRUN_EN | S3C64XX_SPI_INT_RX_UNDERRUN_EN | > +S3C64XX_SPI_INT_TX_OVERRUN_EN | S3C64XX_SPI_INT_TX_UNDERRUN_EN, > +sdd->regs + S3C64XX_SPI_INT_EN); > + > if (spi_register_master(master)) { > dev_err(&pdev->dev, "cannot register SPI master\n"); > ret = -EBUSY; > - goto err8; > + goto err9; > } > > dev_dbg(&pdev->dev, "Samsung SoC SPI Driver loaded for Bus SPI-%d " > @@ -1119,6 +1166,8 @@ static int __init s3c64xx_spi_probe(struct > platform_device *pdev) > > return 0; > > +err9: > + free_irq(irq, sdd); > err8: > destroy_workqueue(sdd->workqueue); > err7: > @@ -1157,6 +1206,10 @@ static int s3c64xx_spi_remove(struct platform_device > *pdev) > > spi_unregister_master(master); > > + writel(0, sdd->regs + S3C64XX_