Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions
On 03/18/2013 06:34 PM, Heiko Stübner wrote:
> Am Montag, 18. März 2013, 23:14:52 schrieb Rob Herring:
>> On 03/18/2013 11:53 AM, Heiko Stübner wrote:
>>> Hi Rob,
>>>
>>> Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
On 03/17/2013 08:09 AM, Heiko Stübner wrote:
> The s3c2450 is special in that it shares the cpu identification with
> the s3c2416 but provides more interrupts for its additional
> components.
>
> It also shares the layout of the main interrupt register with the
> s3c2443 and therefore reuses this definition.
>
> As no non-dt boards are present, the s3c2450 irqs will only be
> accessible thru devicetree.
>>
>> [snip]
>>
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */
This all seems like information that should come from DT.
>>>
>>> In the first iterations [0] of theis series it was done this way, but was
>>> suggested that these informations _might_ be implementation specific and
>>> not describing the hardware.
>>>
>>> As I didn't get any "final" feedback on the matter, I tried it this way
>>> this time. Personally I also did like the previous variant better, as
>>> the driver could loose all the declaration stuff when platforms move to
>>> dt.
>>>
>>> I would be glad for a hint if the first approach was the correct one.
>>>
>>>
>>> [0] "irqchip: irq-s3c24xx: add devicetree support" from 2013-02-18, also
>>> with you in the recipient list
>>
>> I'm inclined to say the prior way is more in the right direction.
>> However, I'm not really clear what you are trying to describe.
>>
>>> + s3c24xx,irqlist = <2 0 /* 2D */
>>> + 2 0 /* IIC1 */
>>> + 0 0 /* reserved */
>>> + 0 0 /* reserved */
>>> + 2 0 /* PCM0 */
>>> + 2 0 /* PCM1 */
>>> + 2 0 /* I2S0 */
>>> + 2 0>; /* I2S1 */
>
> The s3c24xx SoCs contain one (or two) main interrupt controllers. That are
> the
> nodes that gets checked by handle_irq. Additionally they contain something
> called a sub-register which provides more specific irq for some of them.
>
> The list you quoted, is meant to describe which bits of the interrupt
> register
> contain a valid interrupt at all (!= 0), which handler should be used (2 for
> the edge handler, 3 for level handler). The second argument contains the bit
> in the parent interrupt register that contains the parent interrupt, that
> gets
> checked in the irq_entry function.
>
> The original code (which I reworked into this more declarative form)
> distinguished very much between using the edge handler for some and the level
> handler for other interrupts.
>
> This list is essentially the same representation of the list you quoted above
> and which also can be found in [0]
>
> Going this way makes it easy to map datasheet values to interrupt number.
> When
> I read the ac97 interrupt is in bit 28 of the sub-register I can simply
> reference it as
> interrupt-parent = <&subintc>;
> interrupts = <28>;
>
>
> So these lists only desribe the internal structure of the interrupt
> controller
> registers, which vary for each s3c24xx variant.
>
>
>> My first thought here is this information should not be centralized in
>> the controller node, but placed with each source node (2D, I2C1, etc).
>
> I'm not sure I can follow currently :-)
>
> I'll try an example:
>
> The s3c serial driver expects the interrupts for uart tx and rx and the dt
> entry would look like:
>
> serial@5000 {
> compatible = "samsung,s3c2410-uart";
> reg = <0x5000 0x4000>;
> interrupt-parent = <&subintc>;
> interrupts = <0>, <1>;
So what does 0 and 1 correspond to? These are the bits in the subintc?
> };
>
> the map for these in the subintc looks like
>s3c24xx,irqlist = <3 28 /* UART0-RX */
> 3 28 /* UART0-TX */
> 3 28 /* UART0-ERR */
>
> marking them as using the level-handler and being children of the interrupt
> in
> bit 28 of the intc handler.
>
> So the irq_entry would check the intc, see the waiting interrupt in bit 28,
> jump to the demux function which would then handle which ever of rx,tx
Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions
Am Montag, 18. März 2013, 23:14:52 schrieb Rob Herring:
> On 03/18/2013 11:53 AM, Heiko Stübner wrote:
> > Hi Rob,
> >
> > Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
> >> On 03/17/2013 08:09 AM, Heiko Stübner wrote:
> >>> The s3c2450 is special in that it shares the cpu identification with
> >>> the s3c2416 but provides more interrupts for its additional
> >>> components.
> >>>
> >>> It also shares the layout of the main interrupt register with the
> >>> s3c2443 and therefore reuses this definition.
> >>>
> >>> As no non-dt boards are present, the s3c2450 irqs will only be
> >>> accessible thru devicetree.
>
> [snip]
>
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
> >>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */
> >>
> >> This all seems like information that should come from DT.
> >
> > In the first iterations [0] of theis series it was done this way, but was
> > suggested that these informations _might_ be implementation specific and
> > not describing the hardware.
> >
> > As I didn't get any "final" feedback on the matter, I tried it this way
> > this time. Personally I also did like the previous variant better, as
> > the driver could loose all the declaration stuff when platforms move to
> > dt.
> >
> > I would be glad for a hint if the first approach was the correct one.
> >
> >
> > [0] "irqchip: irq-s3c24xx: add devicetree support" from 2013-02-18, also
> > with you in the recipient list
>
> I'm inclined to say the prior way is more in the right direction.
> However, I'm not really clear what you are trying to describe.
>
> > + s3c24xx,irqlist = <2 0 /* 2D */
> > + 2 0 /* IIC1 */
> > + 0 0 /* reserved */
> > + 0 0 /* reserved */
> > + 2 0 /* PCM0 */
> > + 2 0 /* PCM1 */
> > + 2 0 /* I2S0 */
> > + 2 0>; /* I2S1 */
The s3c24xx SoCs contain one (or two) main interrupt controllers. That are the
nodes that gets checked by handle_irq. Additionally they contain something
called a sub-register which provides more specific irq for some of them.
The list you quoted, is meant to describe which bits of the interrupt register
contain a valid interrupt at all (!= 0), which handler should be used (2 for
the edge handler, 3 for level handler). The second argument contains the bit
in the parent interrupt register that contains the parent interrupt, that gets
checked in the irq_entry function.
The original code (which I reworked into this more declarative form)
distinguished very much between using the edge handler for some and the level
handler for other interrupts.
This list is essentially the same representation of the list you quoted above
and which also can be found in [0]
Going this way makes it easy to map datasheet values to interrupt number. When
I read the ac97 interrupt is in bit 28 of the sub-register I can simply
reference it as
interrupt-parent = <&subintc>;
interrupts = <28>;
So these lists only desribe the internal structure of the interrupt controller
registers, which vary for each s3c24xx variant.
> My first thought here is this information should not be centralized in
> the controller node, but placed with each source node (2D, I2C1, etc).
I'm not sure I can follow currently :-)
I'll try an example:
The s3c serial driver expects the interrupts for uart tx and rx and the dt
entry would look like:
serial@5000 {
compatible = "samsung,s3c2410-uart";
reg = <0x5000 0x4000>;
interrupt-parent = <&subintc>;
interrupts = <0>, <1>;
};
the map for these in the subintc looks like
s3c24xx,irqlist = <3 28 /* UART0-RX */
3 28 /* UART0-TX */
3 28 /* UART0-ERR */
marking them as using the level-handler and being children of the interrupt in
bit 28 of the intc handler.
So the irq_entry would check the intc, see the waiting interrupt in bit 28,
jump to the demux function which would then handle which ever of rx,tx or err
would be waiting, which then get sent to the serial driver.
These mappings between sub- and parent irqs also vary very much between the
different s3c24xx variants, as can be seen by the multitude of lists in [0]
and
Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name
On 03/18/2013 04:27 PM, Rob Herring wrote: > On 03/18/2013 01:11 PM, Stephen Warren wrote: >> On 03/18/2013 09:50 AM, Rob Herring wrote: >>> On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote: Rob, On 03/13/2013 03:39 PM, Rob Herring wrote: > I fail to see what the hack is. The order of interrupt properties must > be defined by the binding. interrupt-names is auxiliary data and must > not be required by an OS. >> >> Is that true for all foo-names properties, or only for interrupt-names? >> I was under the impression that foo-names was specifically invented so >> that the order of the entries didn't matter, and instead they could be >> requested by name. > > I think it depends on the specific name the property is tied too. For > interrupt and reg properties which have a long history and convention, > the order should be defined. IIRC, this was Grant's position too. For > new bindings, perhaps we can be more lenient. OK, that makes sense for interrupts/reg. Can we decide that clock-namess are new-style and that order is not significant? I guess gpio-names too? I guess this should be documented in whatever binding describes the core interrupts/reg-names/gpio-names/clock-names/dma-names properties. -- 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] ARM: dts: add interrupt-names property to get interrupt resource by name
On Monday 18 March 2013 17:27:35 Rob Herring wrote: > > I think it depends on the specific name the property is tied too. For > interrupt and reg properties which have a long history and convention, > the order should be defined. IIRC, this was Grant's position too. For > new bindings, perhaps we can be more lenient. At least in case of the dmaengine binding, order is not significant and cannot be, because you may have multiple DMA specifiers referring to the a output of the slave device connected to multiple dma engines, and we require that they all use the same name in that case. Arnd -- 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] ARM: dts: add interrupt-names property to get interrupt resource by name
On 03/18/2013 01:11 PM, Stephen Warren wrote: > On 03/18/2013 09:50 AM, Rob Herring wrote: >> On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote: >>> Rob, >>> >>> On 03/13/2013 03:39 PM, Rob Herring wrote: I fail to see what the hack is. The order of interrupt properties must be defined by the binding. interrupt-names is auxiliary data and must not be required by an OS. > > Is that true for all foo-names properties, or only for interrupt-names? > I was under the impression that foo-names was specifically invented so > that the order of the entries didn't matter, and instead they could be > requested by name. I think it depends on the specific name the property is tied too. For interrupt and reg properties which have a long history and convention, the order should be defined. IIRC, this was Grant's position too. For new bindings, perhaps we can be more lenient. Rob -- 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 v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions
On Monday 18 March 2013 17:14:52 Rob Herring wrote: > > > + s3c24xx,irqlist = <2 0 /* 2D */ > > +2 0 /* IIC1 */ > > +0 0 /* reserved */ > > +0 0 /* reserved */ > > +2 0 /* PCM0 */ > > +2 0 /* PCM1 */ > > +2 0 /* I2S0 */ > > +2 0>; /* I2S1 */ > > My first thought here is this information should not be centralized in > the controller node, but placed with each source node (2D, I2C1, etc). Seconded. Let's do this the same way we have it for all other irq chips and put it all into the irq descriptor referring to the controller, not the controller. Arnd -- 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 v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions
On 03/18/2013 11:53 AM, Heiko Stübner wrote:
> Hi Rob,
>
> Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
>> On 03/17/2013 08:09 AM, Heiko Stübner wrote:
>>> The s3c2450 is special in that it shares the cpu identification with the
>>> s3c2416 but provides more interrupts for its additional components.
>>>
>>> It also shares the layout of the main interrupt register with the s3c2443
>>> and therefore reuses this definition.
>>>
>>> As no non-dt boards are present, the s3c2450 irqs will only be
>>> accessible thru devicetree.
[snip]
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
>>> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */
>>
>> This all seems like information that should come from DT.
>
> In the first iterations [0] of theis series it was done this way, but was
> suggested that these informations _might_ be implementation specific and not
> describing the hardware.
>
> As I didn't get any "final" feedback on the matter, I tried it this way this
> time. Personally I also did like the previous variant better, as the driver
> could loose all the declaration stuff when platforms move to dt.
>
> I would be glad for a hint if the first approach was the correct one.
>
>
> [0] "irqchip: irq-s3c24xx: add devicetree support" from 2013-02-18, also with
> you in the recipient list
I'm inclined to say the prior way is more in the right direction.
However, I'm not really clear what you are trying to describe.
> + s3c24xx,irqlist = <2 0 /* 2D */
> +2 0 /* IIC1 */
> +0 0 /* reserved */
> +0 0 /* reserved */
> +2 0 /* PCM0 */
> +2 0 /* PCM1 */
> +2 0 /* I2S0 */
> +2 0>; /* I2S1 */
My first thought here is this information should not be centralized in
the controller node, but placed with each source node (2D, I2C1, etc).
Rob
--
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/6] pinctrl: samsung: Protect bank registers with a spinlock
Certain pin control registers can be accessed from different contexts,
i.e. pinctrl, gpio and irq functions. This makes the locking provided by
pin control core insufficient.
This patch adds necessary locking using a per bank spinlock as it was
done in the old Samsung GPIO driver.
Signed-off-by: Tomasz Figa
---
drivers/pinctrl/pinctrl-exynos.c | 11 +++
drivers/pinctrl/pinctrl-samsung.c | 24
drivers/pinctrl/pinctrl-samsung.h | 2 ++
3 files changed, 37 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index 538b9dd..cf7700e 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
@@ -81,6 +82,7 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd,
unsigned int type)
unsigned int shift = EXYNOS_EINT_CON_LEN * pin;
unsigned int con, trig_type;
unsigned long reg_con = ctrl->geint_con + bank->eint_offset;
+ unsigned long flags;
unsigned int mask;
switch (type) {
@@ -118,11 +120,15 @@ static int exynos_gpio_irq_set_type(struct irq_data
*irqd, unsigned int type)
shift = pin * bank->func_width;
mask = (1 << bank->func_width) - 1;
+ spin_lock_irqsave(&bank->slock, flags);
+
con = readl(d->virt_base + reg_con);
con &= ~(mask << shift);
con |= EXYNOS_EINT_FUNC << shift;
writel(con, d->virt_base + reg_con);
+ spin_unlock_irqrestore(&bank->slock, flags);
+
return 0;
}
@@ -258,6 +264,7 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd,
unsigned int type)
unsigned long reg_con = d->ctrl->weint_con + bank->eint_offset;
unsigned long shift = EXYNOS_EINT_CON_LEN * pin;
unsigned long con, trig_type;
+ unsigned long flags;
unsigned int mask;
switch (type) {
@@ -295,11 +302,15 @@ static int exynos_wkup_irq_set_type(struct irq_data
*irqd, unsigned int type)
shift = pin * bank->func_width;
mask = (1 << bank->func_width) - 1;
+ spin_lock_irqsave(&bank->slock, flags);
+
con = readl(d->virt_base + reg_con);
con &= ~(mask << shift);
con |= EXYNOS_EINT_FUNC << shift;
writel(con, d->virt_base + reg_con);
+ spin_unlock_irqrestore(&bank->slock, flags);
+
return 0;
}
diff --git a/drivers/pinctrl/pinctrl-samsung.c
b/drivers/pinctrl/pinctrl-samsung.c
index 3475b92..b1d4ac8 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "core.h"
#include "pinctrl-samsung.h"
@@ -289,6 +290,7 @@ static void samsung_pinmux_setup(struct pinctrl_dev
*pctldev, unsigned selector,
struct samsung_pin_bank *bank;
void __iomem *reg;
u32 mask, shift, data, pin_offset, cnt;
+ unsigned long flags;
drvdata = pinctrl_dev_get_drvdata(pctldev);
pins = drvdata->pin_groups[group].pins;
@@ -303,11 +305,15 @@ static void samsung_pinmux_setup(struct pinctrl_dev
*pctldev, unsigned selector,
mask = (1 << bank->func_width) - 1;
shift = pin_offset * bank->func_width;
+ spin_lock_irqsave(&bank->slock, flags);
+
data = readl(reg);
data &= ~(mask << shift);
if (enable)
data |= drvdata->pin_groups[group].func << shift;
writel(data, reg);
+
+ spin_unlock_irqrestore(&bank->slock, flags);
}
}
@@ -338,6 +344,7 @@ static int samsung_pinmux_gpio_set_direction(struct
pinctrl_dev *pctldev,
struct samsung_pinctrl_drv_data *drvdata;
void __iomem *reg;
u32 data, pin_offset, mask, shift;
+ unsigned long flags;
bank = gc_to_pin_bank(range->gc);
drvdata = pinctrl_dev_get_drvdata(pctldev);
@@ -348,11 +355,16 @@ static int samsung_pinmux_gpio_set_direction(struct
pinctrl_dev *pctldev,
mask = (1 << bank->func_width) - 1;
shift = pin_offset * bank->func_width;
+ spin_lock_irqsave(&bank->slock, flags);
+
data = readl(reg);
data &= ~(mask << shift);
if (!input)
data |= FUNC_OUTPUT << shift;
writel(data, reg);
+
+ spin_unlock_irqrestore(&bank->slock, flags);
+
return 0;
}
@@ -376,6 +388,7 @@ static int samsung_pinconf_rw(struct pinctrl_dev *pctldev,
unsigned int pin,
enum pincfg_type cfg_type = PINCFG_UNPACK_TYPE(*config);
u32 data, width, pin_offset, mask, shift;
u32 cfg_value, cfg_reg;
+ unsigned long flags;
drvdata = pinctrl_dev_get_drvdata(pctldev);
pin_to_reg_bank(drvdata, pin - drvdata->ctrl->base, ®_base,
@@ -406,6 +419,8 @@ static int samsung_pinconf_rw(struct pinctrl_dev *pctldev,
unsigned int pin,
if (!width)
Re: [PATCH 2/6] pinctrl: samsung: Include pinctrl-exynos driver data conditionally
On Monday 18 of March 2013 22:31:51 Tomasz Figa wrote:
> Since pinctrl-samsung is a common part of the pin control support for
> several Samsung SoCs, it can be compiled without Exynos support enabled.
>
> This patch surrounds Exynos-specific driver data with ifdefs to include
> them only when support for Exynos is enabled.
>
> Signed-off-by: Tomasz Figa
> ---
> drivers/pinctrl/pinctrl-samsung.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-samsung.c
> b/drivers/pinctrl/pinctrl-samsung.c index b1d4ac8..f1fb562 100644
> --- a/drivers/pinctrl/pinctrl-samsung.c
> +++ b/drivers/pinctrl/pinctrl-samsung.c
> @@ -968,10 +968,12 @@ static int samsung_pinctrl_probe(struct
> platform_device *pdev) }
>
> static const struct of_device_id samsung_pinctrl_dt_match[] = {
> +#ifdef CONFIG_PINCTRL_EXYNOS4
Oops... This should be CONFIG_PINCTRL_EXYNOS. It used to be EXYNOS4 in
Kgene's tree (on which I usually base my patches) for some time, but it no
longer is, but I forgot to change this during rebae.
Linus, if remaining patches turned out to be OK, would you fix this when
applying?
Best regards,
Tomasz
--
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/6] pinctrl: samsung: Include pinctrl-exynos driver data conditionally
Since pinctrl-samsung is a common part of the pin control support for
several Samsung SoCs, it can be compiled without Exynos support enabled.
This patch surrounds Exynos-specific driver data with ifdefs to include
them only when support for Exynos is enabled.
Signed-off-by: Tomasz Figa
---
drivers/pinctrl/pinctrl-samsung.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-samsung.c
b/drivers/pinctrl/pinctrl-samsung.c
index b1d4ac8..f1fb562 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -968,10 +968,12 @@ static int samsung_pinctrl_probe(struct platform_device
*pdev)
}
static const struct of_device_id samsung_pinctrl_dt_match[] = {
+#ifdef CONFIG_PINCTRL_EXYNOS4
{ .compatible = "samsung,exynos4210-pinctrl",
.data = (void *)exynos4210_pin_ctrl },
{ .compatible = "samsung,exynos4x12-pinctrl",
.data = (void *)exynos4x12_pin_ctrl },
+#endif
{},
};
MODULE_DEVICE_TABLE(of, samsung_pinctrl_dt_match);
--
1.8.1.5
--
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 3/6] pinctrl: samsung: Split pin bank description into two structures
This patch splits pin bank description into two structures, one
describing bank type (currently only bitfield widths), which can be
shared across multiple banks and second containing bank-specific
parameters including a pointer to a bank type struct.
It is a prerequisite for further patch removing the statically hardcoded
register offsets, making it impossible to support SoCs with different
set and order of pin control registers.
Signed-off-by: Tomasz Figa
---
drivers/pinctrl/pinctrl-exynos.c | 19 +++
drivers/pinctrl/pinctrl-exynos.h | 16 +++-
drivers/pinctrl/pinctrl-samsung.c | 24 +++-
drivers/pinctrl/pinctrl-samsung.h | 26 --
4 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index cf7700e..b5dbb87 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -34,6 +34,15 @@
#include "pinctrl-samsung.h"
#include "pinctrl-exynos.h"
+
+static struct samsung_pin_bank_type bank_type_off = {
+ .fld_width = { 4, 1, 2, 2, 2, 2, },
+};
+
+static struct samsung_pin_bank_type bank_type_alive = {
+ .fld_width = { 4, 1, 2, 2, },
+};
+
/* list of external wakeup controllers supported */
static const struct of_device_id exynos_wkup_irq_ids[] = {
{ .compatible = "samsung,exynos4210-wakeup-eint", },
@@ -76,6 +85,7 @@ static void exynos_gpio_irq_ack(struct irq_data *irqd)
static int exynos_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
{
struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+ struct samsung_pin_bank_type *bank_type = bank->type;
struct samsung_pinctrl_drv_data *d = bank->drvdata;
struct samsung_pin_ctrl *ctrl = d->ctrl;
unsigned int pin = irqd->hwirq;
@@ -117,8 +127,8 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd,
unsigned int type)
writel(con, d->virt_base + reg_con);
reg_con = bank->pctl_offset;
- shift = pin * bank->func_width;
- mask = (1 << bank->func_width) - 1;
+ shift = pin * bank_type->fld_width[PINCFG_TYPE_FUNC];
+ mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
spin_lock_irqsave(&bank->slock, flags);
@@ -259,6 +269,7 @@ static void exynos_wkup_irq_ack(struct irq_data *irqd)
static int exynos_wkup_irq_set_type(struct irq_data *irqd, unsigned int type)
{
struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+ struct samsung_pin_bank_type *bank_type = bank->type;
struct samsung_pinctrl_drv_data *d = bank->drvdata;
unsigned int pin = irqd->hwirq;
unsigned long reg_con = d->ctrl->weint_con + bank->eint_offset;
@@ -299,8 +310,8 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd,
unsigned int type)
writel(con, d->virt_base + reg_con);
reg_con = bank->pctl_offset;
- shift = pin * bank->func_width;
- mask = (1 << bank->func_width) - 1;
+ shift = pin * bank_type->fld_width[PINCFG_TYPE_FUNC];
+ mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
spin_lock_irqsave(&bank->slock, flags);
diff --git a/drivers/pinctrl/pinctrl-exynos.h b/drivers/pinctrl/pinctrl-exynos.h
index 0a70889..9b1f77a 100644
--- a/drivers/pinctrl/pinctrl-exynos.h
+++ b/drivers/pinctrl/pinctrl-exynos.h
@@ -48,26 +48,18 @@
#define EXYNOS_PIN_BANK_EINTN(pins, reg, id) \
{ \
+ .type = &bank_type_off, \
.pctl_offset= reg, \
.nr_pins= pins, \
- .func_width = 4,\
- .pud_width = 2,\
- .drv_width = 2,\
- .conpdn_width = 2,\
- .pudpdn_width = 2,\
.eint_type = EINT_TYPE_NONE, \
.name = id\
}
#define EXYNOS_PIN_BANK_EINTG(pins, reg, id, offs) \
{ \
+ .type = &bank_type_off, \
.pctl_offset= reg, \
.nr_pins= pins, \
- .func_width = 4,\
- .pud_width = 2,\
- .drv_width = 2,\
- .conpdn_width = 2,\
- .pudpdn_width = 2,\
.eint_type = EINT_TYPE_GPIO, \
.eint_offset= offs, \
.name = id\
@@ -75,11 +67,9 @@
#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \
{
[PATCH 4/6] pinctrl: samsung: Remove hardcoded register offsets
This patch replaces statically hardcoded register offsets of Exynos SoCs
with an array of register offsets in samsung_pin_bank_type struct.
Thanks to this change, support for SoCs with other set and order of
registers can be added (e.g. S3C24xx and S3C64xx).
Signed-off-by: Tomasz Figa
---
drivers/pinctrl/pinctrl-exynos.c | 6 --
drivers/pinctrl/pinctrl-samsung.c | 37 +
drivers/pinctrl/pinctrl-samsung.h | 9 ++---
3 files changed, 15 insertions(+), 37 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index b5dbb87..8b10b1a 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -37,10 +37,12 @@
static struct samsung_pin_bank_type bank_type_off = {
.fld_width = { 4, 1, 2, 2, 2, 2, },
+ .reg_offset = { 0x00, 0x04, 0x08, 0x0c, 0x10, 0x14, },
};
static struct samsung_pin_bank_type bank_type_alive = {
.fld_width = { 4, 1, 2, 2, },
+ .reg_offset = { 0x00, 0x04, 0x08, 0x0c, },
};
/* list of external wakeup controllers supported */
@@ -126,7 +128,7 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd,
unsigned int type)
con |= trig_type << shift;
writel(con, d->virt_base + reg_con);
- reg_con = bank->pctl_offset;
+ reg_con = bank->pctl_offset + bank_type->reg_offset[PINCFG_TYPE_FUNC];
shift = pin * bank_type->fld_width[PINCFG_TYPE_FUNC];
mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
@@ -309,7 +311,7 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd,
unsigned int type)
con |= trig_type << shift;
writel(con, d->virt_base + reg_con);
- reg_con = bank->pctl_offset;
+ reg_con = bank->pctl_offset + bank_type->reg_offset[PINCFG_TYPE_FUNC];
shift = pin * bank_type->fld_width[PINCFG_TYPE_FUNC];
mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
diff --git a/drivers/pinctrl/pinctrl-samsung.c
b/drivers/pinctrl/pinctrl-samsung.c
index ac428ff..45a9939 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -275,10 +275,6 @@ static void pin_to_reg_bank(struct
samsung_pinctrl_drv_data *drvdata,
*offset = pin - b->pin_base;
if (bank)
*bank = b;
-
- /* some banks have two config registers in a single bank */
- if (*offset * b->func_width > BITS_PER_LONG)
- *reg += 4;
}
/* enable or disable a pinmux function */
@@ -310,11 +306,11 @@ static void samsung_pinmux_setup(struct pinctrl_dev
*pctldev, unsigned selector,
spin_lock_irqsave(&bank->slock, flags);
- data = readl(reg);
+ data = readl(reg + type->reg_offset[PINCFG_TYPE_FUNC]);
data &= ~(mask << shift);
if (enable)
data |= drvdata->pin_groups[group].func << shift;
- writel(data, reg);
+ writel(data, reg + type->reg_offset[PINCFG_TYPE_FUNC]);
spin_unlock_irqrestore(&bank->slock, flags);
}
@@ -355,7 +351,8 @@ static int samsung_pinmux_gpio_set_direction(struct
pinctrl_dev *pctldev,
drvdata = pinctrl_dev_get_drvdata(pctldev);
pin_offset = offset - bank->pin_base;
- reg = drvdata->virt_base + bank->pctl_offset;
+ reg = drvdata->virt_base + bank->pctl_offset +
+ type->reg_offset[PINCFG_TYPE_FUNC];
mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1;
shift = pin_offset * type->fld_width[PINCFG_TYPE_FUNC];
@@ -401,28 +398,11 @@ static int samsung_pinconf_rw(struct pinctrl_dev
*pctldev, unsigned int pin,
&pin_offset, &bank);
type = bank->type;
- switch (cfg_type) {
- case PINCFG_TYPE_PUD:
- cfg_reg = PUD_REG;
- break;
- case PINCFG_TYPE_DRV:
- cfg_reg = DRV_REG;
- break;
- case PINCFG_TYPE_CON_PDN:
- cfg_reg = CONPDN_REG;
- break;
- case PINCFG_TYPE_PUD_PDN:
- cfg_reg = PUDPDN_REG;
- break;
- default:
- WARN_ON(1);
- return -EINVAL;
- }
-
if (cfg_type >= PINCFG_TYPE_NUM || !type->fld_width[cfg_type])
return -EINVAL;
width = type->fld_width[cfg_type];
+ cfg_reg = type->reg_offset[cfg_type];
spin_lock_irqsave(&bank->slock, flags);
@@ -511,11 +491,11 @@ static void samsung_gpio_set(struct gpio_chip *gc,
unsigned offset, int value)
spin_lock_irqsave(&bank->slock, flags);
- data = readl(reg + DAT_REG);
+ data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]);
data &= ~(1 << offset);
if (value)
data |= 1 << offset;
- writel(data, reg + DAT_REG);
+ writel(data, reg + type->reg_offset[PINCFG_TYPE_DAT]);
s
[PATCH 6/6] pinctrl: Add pinctrl-s3c64xx driver
This patch adds pinctrl-s3c64xx driver which implements pin control interface for Samsung S3C64xx SoCs. Signed-off-by: Tomasz Figa --- .../bindings/pinctrl/samsung-pinctrl.txt | 3 + drivers/pinctrl/Kconfig| 5 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-s3c64xx.c | 817 + drivers/pinctrl/pinctrl-samsung.c | 4 + drivers/pinctrl/pinctrl-samsung.h | 5 + 6 files changed, 835 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-s3c64xx.c diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt index 4598a47..c70fca1 100644 --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt @@ -7,6 +7,7 @@ on-chip controllers onto these pads. Required Properties: - compatible: should be one of the following. + - "samsung,s3c64xx-pinctrl": for S3C64xx-compatible pin-controller, - "samsung,exynos4210-pinctrl": for Exynos4210 compatible pin-controller. - "samsung,exynos4x12-pinctrl": for Exynos4x12 compatible pin-controller. - "samsung,exynos5250-pinctrl": for Exynos5250 compatible pin-controller. @@ -105,6 +106,8 @@ B. External Wakeup Interrupts: For supporting external wakeup interrupts, a - compatible: identifies the type of the external wakeup interrupt controller The possible values are: + - samsung,s3c64xx-wakeup-eint: represents wakeup interrupt controller + found on Samsung S3C64xx SoCs, - samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller found on Samsung Exynos4210 SoC. - interrupt-parent: phandle of the interrupt parent to which the external diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 5a690ce..b1cc5dc 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -227,6 +227,11 @@ config PINCTRL_EXYNOS5440 select PINMUX select PINCONF +config PINCTRL_S3C64XX + bool "Samsung S3C64XX SoC pinctrl driver" + depends on ARCH_S3C64XX + select PINCTRL_SAMSUNG + source "drivers/pinctrl/mvebu/Kconfig" source "drivers/pinctrl/sh-pfc/Kconfig" source "drivers/pinctrl/spear/Kconfig" diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index f82cc5b..21d34c2 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_COH901) += pinctrl-coh901.o obj-$(CONFIG_PINCTRL_SAMSUNG) += pinctrl-samsung.o obj-$(CONFIG_PINCTRL_EXYNOS) += pinctrl-exynos.o obj-$(CONFIG_PINCTRL_EXYNOS5440) += pinctrl-exynos5440.o +obj-$(CONFIG_PINCTRL_S3C64XX) += pinctrl-s3c64xx.o obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o obj-$(CONFIG_PINCTRL_LANTIQ) += pinctrl-lantiq.o diff --git a/drivers/pinctrl/pinctrl-s3c64xx.c b/drivers/pinctrl/pinctrl-s3c64xx.c new file mode 100644 index 000..b5d1c4a --- /dev/null +++ b/drivers/pinctrl/pinctrl-s3c64xx.c @@ -0,0 +1,817 @@ +/* + * S3C64xx specific support for pinctrl-samsung driver. + * + * Copyright (c) 2013 Tomasz Figa + * + * Based on pinctrl-exynos.c, please see the file for original copyrights. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This file contains the Samsung S3C64xx specific information required by the + * the Samsung pinctrl/gpiolib driver. It also includes the implementation of + * external gpio and wakeup interrupt support. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "pinctrl-samsung.h" + +#define NUM_EINT0 28 +#define NUM_EINT0_IRQ 4 +#define EINT_MAX_PER_REG 16 +#define EINT_MAX_PER_GROUP 16 + +/* External GPIO and wakeup interrupt related definitions */ +#define SVC_GROUP_SHIFT4 +#define SVC_GROUP_MASK 0xf +#define SVC_NUM_MASK 0xf +#define SVC_GROUP(x) ((x >> SVC_GROUP_SHIFT) & \ + SVC_GROUP_MASK) + +#define EINT12CON_REG 0x200 +#define EINT12MASK_REG 0x240 +#define EINT12PEND_REG 0x260 + +#define EINT_OFFS(i) ((i) % (2 * EINT_MAX_PER_GROUP)) +#define EINT_GROUP(i) ((i) / EINT_MAX_PER_GROUP) +#define EINT_REG(g)(4 * ((g) / 2)) + +#define EINTCON_REG(i) (EINT12CON_REG + EINT_REG(EINT_GROUP(i))) +#define EINTMASK_REG(i)(EINT12MASK_REG + EINT_REG(EINT_GROUP(i))) +#define EINTPEND_REG(i)(EINT12PEND_REG + EINT_REG(EINT_GROUP(i))) + +#define SERVICE_REG0x284 +#define SERVICEPEND_REG0x288 + +#defin
[PATCH 5/6] pinctrl: samsung: Handle banks with two configuration registers
This patch adds support for banks that have more than one function
configuration registers, e.g. some of the banks of S3C64xx SoCs.
Signed-off-by: Tomasz Figa
---
drivers/pinctrl/pinctrl-samsung.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-samsung.c
b/drivers/pinctrl/pinctrl-samsung.c
index 45a9939..b738b7b 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -303,6 +303,11 @@ static void samsung_pinmux_setup(struct pinctrl_dev
*pctldev, unsigned selector,
type = bank->type;
mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1;
shift = pin_offset * type->fld_width[PINCFG_TYPE_FUNC];
+ if (shift >= 32) {
+ /* Some banks have two config registers */
+ shift -= 32;
+ reg += 4;
+ }
spin_lock_irqsave(&bank->slock, flags);
@@ -356,6 +361,11 @@ static int samsung_pinmux_gpio_set_direction(struct
pinctrl_dev *pctldev,
mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1;
shift = pin_offset * type->fld_width[PINCFG_TYPE_FUNC];
+ if (shift >= 32) {
+ /* Some banks have two config registers */
+ shift -= 32;
+ reg += 4;
+ }
spin_lock_irqsave(&bank->slock, flags);
--
1.8.1.5
--
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/6] pinctrl: Add support for pin control on S3C64xx
This series makes necessary preparations to add support for pin controller available on Samsung S3C64xx using pinctrl-samsung driver and then adds pinctrl-s3c64xx driver which implements SoC-specific part of the code. It has been tested on a tiny6410 (mini6410-compatible) board with my patches for samsung-time cleanup and S3C64xx Device Tree support: - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/16664 - http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg16325.html Just the driver is added for now, as this is the part that can safely go through Linus' pinctrl tree. I will post appropriate enablement patches after all the dependencies get merged to Kgene's tree. See particular patches for more detailed description of all changes. Tomasz Figa (6): pinctrl: samsung: Protect bank registers with a spinlock pinctrl: samsung: Include pinctrl-exynos driver data conditionally pinctrl: samsung: Split pin bank description into two structures pinctrl: samsung: Remove hardcoded register offsets pinctrl: samsung: Handle banks with two configuration registers pinctrl: Add pinctrl-s3c64xx driver .../bindings/pinctrl/samsung-pinctrl.txt | 3 + drivers/pinctrl/Kconfig| 5 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-exynos.c | 36 +- drivers/pinctrl/pinctrl-exynos.h | 16 +- drivers/pinctrl/pinctrl-s3c64xx.c | 817 + drivers/pinctrl/pinctrl-samsung.c | 99 ++- drivers/pinctrl/pinctrl-samsung.h | 42 +- 8 files changed, 947 insertions(+), 72 deletions(-) create mode 100644 drivers/pinctrl/pinctrl-s3c64xx.c -- 1.8.1.5 -- 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] [media] Add a V4L2 OF parser
From: Guennadi Liakhovetski
Add a V4L2 OF parser, implementing bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt.
Signed-off-by: Guennadi Liakhovetski
[s.nawro...@samsung.com: various corrections and improvements
since the initial version]
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
Geunnadi,
I have made couple further changes to this parsing library, mostly
addressing your review comments. Hopefully it looks better now.
It would be good to get more reviews and opinions of other media
sub-maintainers. Any Acked/Tested-by are also of course most
welcome.
I have a feeling this is about ready for upstream ;-) We have
used it in couple subsystems already and the overall design
looks good and seems to be proving itself in practice.
Thanks,
Sylwester
The last version of the bindings documentation patch can be found
at: https://patchwork.kernel.org/patch/2074951
Changes since v7 include mostly corrections of issues pointed
out by Guennadi, and few others:
- v4l2_of_parse_parallel_bus() and v4l2_of_parse_csi_bus() are
made static and are not exported anymore;
- swapped order of parsing the serial and parallel bus properties
in v4l2_of_parse_endpoint() function; the rationale is that
MIPI CSI-2 has less properties and an overall number of searches
of non-existent properties is less this way;
- v4l2_mbus_mipi_csi2, v4l2_mbus_parallel structs renamed to
v4l2_of_bus_mipi_csi2, v4l2_of_bus_parallel respectively;
- added kernel-doc for the above two data structures;
- the flags field split into to fields - separate fields for the
parallel and serial bus, V4L2_MBUS_* bit flags are defined
separately for MIPI CSI-2 and parallel busses so they were
clashing when used together in single data structure member!;
- the head member of struct v4l2_endpoint made last member of
this data structure, it is now not cleared with memset() at the
beginning of v4l2_of_parse_endpoint() function; I believe
clearing list head in this function is not expected;
- fixed bug in v4l2_of_get_remote_port_parent() function;
- reworked first port node searching in v4l2_of_get_next_endpoint();
- moved code setting V4L2_MBUS_MASTER and V4L2_MBUS_CSI2_CONTINUOUS_CLOCK
out from v4l2_of_parse_endpoint() to v4l2_of_parse_mipi_csi2/
parallel_bus() functions;
- ensured we recognize presence of the clock-lanes property regardless
of the clock lane index being 0;
- added v4l2_of_get_remote_port() function.
Changes since v6:
- minor v4l2_of_get_remote_port_parent() function cleanup.
Changes since v5:
- renamed v4l2_of_parse_mipi_csi2 -> v4l2_of_parse_csi_bus;
- corrected v4l2_of_get_remote_port_parent() function declaration
for !CONFIG_OF;
- reworked v4l2_of_get_next_endpoint() function to consider the
'port' nodes can be grouped under optional 'ports' node;
- added kerneldoc description for v4l2_of_get_next_endpoint()
function.
Changes since v4:
- reworked v4l2_of_get_remote_port() function to consider cases
where 'port' nodes are grouped in a parent 'ports' node;
- rearranged struct v4l2_of_endpoint and related changes added
in the parser code;
- added kerneldoc description for struct v4l2_of_endpoint;
- s/link/endpoint in the comments.
---
drivers/media/v4l2-core/Makefile |3 +
drivers/media/v4l2-core/v4l2-of.c | 267 +
include/media/v4l2-of.h | 111 +++
3 files changed, 381 insertions(+)
create mode 100644 drivers/media/v4l2-core/v4l2-of.c
create mode 100644 include/media/v4l2-of.h
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o
v4l2-fh.o \
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
+ifeq ($(CONFIG_OF),y)
+ videodev-objs += v4l2-of.o
+endif
obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 000..e38e210
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,267 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Sylwester Nawrocki
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+static void v4l2_of_parse_csi_bus(const struct device_node *node,
+ struct v4l2_of_endpoint *endpoint)
+{
+ struct v4l2_of_bus_mipi_csi2 *bus
Re: [PATCH v3 5/6] irqchip: s3c24xx: add devicetree support
On 03/17/2013 08:07 AM, Heiko Stübner wrote:
> Add the necessary code to initialize the interrupt controller
> thru devicetree data using the irqchip infrastructure.
>
> On dt machines the eint-type interrupts in the main interrupt controller
> get mapped as regular edge-types, as their wakeup and interrupt type
> properties will be handled by the upcoming pinctrl driver.
>
> Signed-off-by: Heiko Stuebner
> ---
> .../interrupt-controller/samsung,s3c24xx-irq.txt | 54 +
> drivers/irqchip/irq-s3c24xx.c | 222
>
> 2 files changed, 276 insertions(+), 0 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
>
> diff --git
> a/Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
>
> b/Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
> new file mode 100644
> index 000..be5dead
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
> @@ -0,0 +1,54 @@
> +Samsung S3C24XX Interrupt Controllers
> +
> +The S3C24XX SoCs contain a custom set of interrupt controllers providing a
> +varying number of interrupt sources. The set consists of a main- and sub-
> +controller and on newer SoCs even a second main controller.
> +
> +Required properties:
> +- compatible: Compatible property value should be one of
> "samsung,s3c2410-irq",
> + "samsung,s3c2412-irq", "samsung,s3c2416-irq", "samsung,s3c2440-irq",
> + "samsung,s3c2442-irq", "samsung,s3c2443-irq" depending on the SoC variant.
> +
> +- reg: Physical base address of the controller and length of memory mapped
> + region.
> +
> +- interrupt-controller : Identifies the node as an interrupt controller
> +
> +Sub-controllers as child nodes:
> + The interrupt controllers that should be referenced by device nodes are
> + represented by child nodes. Valid names are intc, subintc and intc2.
> + The interrupt values in device nodes are then mapped directly to the
> + bit-numbers of the pending register of the named interrupt controller.
> +
> +Required properties:
> +- interrupt-controller : Identifies the node as an interrupt controller
> +
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> + interrupt source. The value shall be 2.
> +
> +Example:
> +
> + interrupt-controller@4a00 {
> + compatible = "samsung,s3c2416-irq";
> + reg = <0x4a00 0x100>;
> + interrupt-controller;
> +
> + intc:intc {
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> +
> + subintc:subintc {
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> + };
> +
> + [...]
> +
> + serial@5000 {
> + compatible = "samsung,s3c2410-uart";
> + reg = <0x5000 0x4000>;
> + interrupt-parent = <&subintc>;
> + interrupts = <0 0>, <1 0>;
> + };
> diff --git a/drivers/irqchip/irq-s3c24xx.c b/drivers/irqchip/irq-s3c24xx.c
> index 1eba289..55cb363 100644
> --- a/drivers/irqchip/irq-s3c24xx.c
> +++ b/drivers/irqchip/irq-s3c24xx.c
> @@ -25,6 +25,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> #include
> #include
> @@ -36,6 +39,8 @@
> #include
> #include
>
> +#include "irqchip.h"
> +
> #define S3C_IRQTYPE_NONE 0
> #define S3C_IRQTYPE_EINT 1
> #define S3C_IRQTYPE_EDGE 2
> @@ -380,6 +385,10 @@ static int s3c24xx_irq_map(struct irq_domain *h,
> unsigned int virq,
>
> parent_intc = intc->parent;
>
> + /* on dt platforms the extints get handled by the pinctrl driver */
> + if (h->of_node && irq_data->type == S3C_IRQTYPE_EINT)
> + irq_data->type = S3C_IRQTYPE_EDGE;
> +
> /* set handler and flags */
> switch (irq_data->type) {
> case S3C_IRQTYPE_NONE:
> @@ -1104,3 +1113,216 @@ void __init s3c2443_init_irq(void)
> s3c24xx_init_intc(NULL, &init_s3c2443subint[0], main_intc, 0x4a18);
> }
> #endif
> +
> +#ifdef CONFIG_OF
> +struct s3c24xx_irq_of_ctrl {
> + char*name;
> + unsigned long offset;
> + struct s3c_irq_data *irq_data;
> + struct s3c_irq_intc **handle;
> + struct s3c_irq_intc **parent;
> +};
> +
> +#define S3C24XX_IRQCTRL(n, o, d, h, p) \
> +{\
> + .name = n,\
> + .offset = o,\
> + .irq_data = d,\
> + .handle = h,\
> + .parent = p,\
> +}
> +
> +struct s3c24xx_irq_of_data {
> + struct s3c24xx_irq_of_ctrl *irq_ctrl;
> + int num_ctrl;
> +};
> +
> +#ifdef CONFIG_CPU_S3C2410
> +s
Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name
On 03/18/2013 09:50 AM, Rob Herring wrote: > On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote: >> Rob, >> >> On 03/13/2013 03:39 PM, Rob Herring wrote: >>> I fail to see what the hack is. The order of interrupt properties must >>> be defined by the binding. interrupt-names is auxiliary data and must >>> not be required by an OS. Is that true for all foo-names properties, or only for interrupt-names? I was under the impression that foo-names was specifically invented so that the order of the entries didn't matter, and instead they could be requested by name. -- 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 v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions
Hi Rob,
Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
> On 03/17/2013 08:09 AM, Heiko Stübner wrote:
> > The s3c2450 is special in that it shares the cpu identification with the
> > s3c2416 but provides more interrupts for its additional components.
> >
> > It also shares the layout of the main interrupt register with the s3c2443
> > and therefore reuses this definition.
> >
> > As no non-dt boards are present, the s3c2450 irqs will only be
> > accessible thru devicetree.
> >
> > Signed-off-by: Heiko Stuebner
> > ---
> >
> > drivers/irqchip/irq-s3c24xx.c | 62
> > +++- 1 files changed, 60
> > insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-s3c24xx.c
> > b/drivers/irqchip/irq-s3c24xx.c index 55cb363..7ddf8e8 100644
> > --- a/drivers/irqchip/irq-s3c24xx.c
> > +++ b/drivers/irqchip/irq-s3c24xx.c
> > @@ -852,6 +852,51 @@ static struct s3c_irq_data init_s3c2416_second[32] =
> > {
> >
> > { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
> >
> > };
> >
> > +static struct s3c_irq_data init_s3c2450subint[32] = {
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-RX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-TX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-ERR */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-RX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-TX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-ERR */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-RX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-TX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-ERR */
> > + { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* TC */
> > + { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* ADC */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_C */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_P */
> > + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> > + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD2 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD3 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD4 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA0 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA1 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA2 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA3 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
> > + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */
>
> This all seems like information that should come from DT.
In the first iterations [0] of theis series it was done this way, but was
suggested that these informations _might_ be implementation specific and not
describing the hardware.
As I didn't get any "final" feedback on the matter, I tried it this way this
time. Personally I also did like the previous variant better, as the driver
could loose all the declaration stuff when platforms move to dt.
I would be glad for a hint if the first approach was the correct one.
[0] "irqchip: irq-s3c24xx: add devicetree support" from 2013-02-18, also with
you in the recipient list
>
> > +};
> > +
> > +static struct s3c_irq_data init_s3c2450second[32] = {
> > + { .type = S3C_IRQTYPE_EDGE }, /* 2D */
> > + { .type = S3C_IRQTYPE_EDGE }, /* IIC1 */
> > + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> > + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> > + { .type = S3C_IRQTYPE_EDGE }, /* PCM0 */
> > + { .type = S3C_IRQTYPE_EDGE }, /* PCM1 */
> > + { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
> > + { .type = S3C_IRQTYPE_EDGE }, /* I2S1 */
> > +};
> > +
> >
> > void __init s3c2416_init_irq(void)
> > {
> >
> > struct s3c_irq_intc *main_intc;
> >
> > @@ -1024,7 +1069,7 @@ void __init s3c2442_init_irq(void)
> >
> > }
> > #endif
> >
> > -#ifdef CONFIG_CPU_S3C2443
> > +#if defined(CONFIG_CPU_S3C2443) || defined(CONFIG_CPU_S3C2416)
> >
> > static struct s3c_irq_data init_s3c2443base[32] = {
> >
> > { .type = S3C_IRQTYPE_EINT, }, /* EINT0 */
> > { .type = S3C_IRQTYPE_EINT, }, /* EINT1 */
> >
> > @@ -1059,8 +1104,9 @@ static struct s3c_irq_data init_s3c2443base[32] = {
> >
> > { .type = S3C_IRQTYPE_EDGE, }, /* RTC */
> > { .type = S3C_IRQTYPE_LEVEL, }, /* ADCPA
Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions
On 03/17/2013 08:09 AM, Heiko Stübner wrote:
> The s3c2450 is special in that it shares the cpu identification with the
> s3c2416 but provides more interrupts for its additional components.
>
> It also shares the layout of the main interrupt register with the s3c2443
> and therefore reuses this definition.
>
> As no non-dt boards are present, the s3c2450 irqs will only be
> accessible thru devicetree.
>
> Signed-off-by: Heiko Stuebner
> ---
> drivers/irqchip/irq-s3c24xx.c | 62 +++-
> 1 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-s3c24xx.c b/drivers/irqchip/irq-s3c24xx.c
> index 55cb363..7ddf8e8 100644
> --- a/drivers/irqchip/irq-s3c24xx.c
> +++ b/drivers/irqchip/irq-s3c24xx.c
> @@ -852,6 +852,51 @@ static struct s3c_irq_data init_s3c2416_second[32] = {
> { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
> };
>
> +static struct s3c_irq_data init_s3c2450subint[32] = {
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-RX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-TX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-ERR */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-RX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-TX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-ERR */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-RX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-TX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-ERR */
> + { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* TC */
> + { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* ADC */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_C */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_P */
> + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD2 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD3 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD4 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA0 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA1 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA2 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA3 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
> + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */
This all seems like information that should come from DT.
> +};
> +
> +static struct s3c_irq_data init_s3c2450second[32] = {
> + { .type = S3C_IRQTYPE_EDGE }, /* 2D */
> + { .type = S3C_IRQTYPE_EDGE }, /* IIC1 */
> + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> + { .type = S3C_IRQTYPE_NONE }, /* reserved */
> + { .type = S3C_IRQTYPE_EDGE }, /* PCM0 */
> + { .type = S3C_IRQTYPE_EDGE }, /* PCM1 */
> + { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
> + { .type = S3C_IRQTYPE_EDGE }, /* I2S1 */
> +};
> +
> void __init s3c2416_init_irq(void)
> {
> struct s3c_irq_intc *main_intc;
> @@ -1024,7 +1069,7 @@ void __init s3c2442_init_irq(void)
> }
> #endif
>
> -#ifdef CONFIG_CPU_S3C2443
> +#if defined(CONFIG_CPU_S3C2443) || defined(CONFIG_CPU_S3C2416)
> static struct s3c_irq_data init_s3c2443base[32] = {
> { .type = S3C_IRQTYPE_EINT, }, /* EINT0 */
> { .type = S3C_IRQTYPE_EINT, }, /* EINT1 */
> @@ -1059,8 +1104,9 @@ static struct s3c_irq_data init_s3c2443base[32] = {
> { .type = S3C_IRQTYPE_EDGE, }, /* RTC */
> { .type = S3C_IRQTYPE_LEVEL, }, /* ADCPARENT */
> };
> +#endif
>
> -
> +#ifdef CONFIG_CPU_S3C2443
> static struct s3c_irq_data init_s3c2443subint[32] = {
> { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-RX */
> { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-TX */
> @@ -1172,6 +1218,17 @@ struct s3c24xx_irq_of_data s3c2416_irq_data = {
> .irq_ctrl = s3c2416_ctrl,
> .num_ctrl = ARRAY_SIZE(s3c2416_ctrl),
> };
> +
> +static struct s3c24xx_irq_of_ctrl s3c2450_ctrl[] = {
> + S3C24XX_IRQCTRL("intc", 0, init_s3c2443base, &main_intc, NULL),
> + S3C24XX_IRQCTRL("subintc", 0x18, init_s3c2450subint, NULL, &main_intc),
> + S3C24XX_IRQCTRL("intc2", 0x40, init_s3c2450second, &main_intc2, NULL),
> +};
> +
> +static struct s3c24xx_irq_of_data s3c2450_irq_data = {
> + .ir
Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name
On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote: > Rob, > > On 03/13/2013 03:39 PM, Rob Herring wrote: >> I fail to see what the hack is. The order of interrupt properties must >> be defined by the binding. interrupt-names is auxiliary data and must >> not be required by an OS. > > It is clear that the order of the interrupts must be defined by the > bindings. But how useful -names properties are when we > cannot define them as required ? If an OS cannot rely on them then > it must use some other, reliable, method to identify the resources, > e.g. by hard coding the indexes. If we have to do it then why even > bother with the -names properties ? I can see interrupt-names > property specified as required in at least 2 bindings' documentation > and all bindings having reg-names property define it as required. > Are they wrong them ? You can require the name for a binding definition, but that does not remove the requirement that the order and index of interrupts also be defined by the binding. Then it is up to the OS to use names or hard-coded indexes. Hard-coded indexes are not a hack. This is how FDT and OF are defined to work. I'm still not clear how changing the order of the interrupts removes a hack. Rob -- 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: [RFC PATCH 5/8] s5p-fimc: Add ISP video capture driver stubs
On Sun March 17 2013 22:03:38 Sylwester Nawrocki wrote:
> On 03/12/2013 03:44 PM, Hans Verkuil wrote:
> > On Mon 11 March 2013 20:44:49 Sylwester Nawrocki wrote:
> [...]
> >> +static int isp_video_capture_open(struct file *file)
> >> +{
> >> + struct fimc_isp *isp = video_drvdata(file);
> >> + int ret = 0;
> >> +
> >> + if (mutex_lock_interruptible(&isp->video_lock))
> >> + return -ERESTARTSYS;
> >> +
> >> + /* ret = pm_runtime_get_sync(&isp->pdev->dev); */
> >> + if (ret< 0)
> >> + goto done;
> >> +
> >> + ret = v4l2_fh_open(file);
> >> + if (ret< 0)
> >> + goto done;
> >> +
> >> + /* TODO: prepare video pipeline */
> >> +done:
> >> + mutex_unlock(&isp->video_lock);
> >> + return ret;
> >> +}
> >> +
> >> +static int isp_video_capture_close(struct file *file)
> >> +{
> >> + struct fimc_isp *isp = video_drvdata(file);
> >> + int ret = 0;
> >> +
> >> + mutex_lock(&isp->video_lock);
> >> +
> >> + if (isp->out_path == FIMC_IO_DMA) {
> >> + /* TODO: stop capture, cleanup */
> >> + }
> >> +
> >> + /* pm_runtime_put(&isp->pdev->dev); */
> >> +
> >> + if (isp->ref_count == 0)
> >> + vb2_queue_release(&isp->capture_vb_queue);
> >> +
> >> + ret = v4l2_fh_release(file);
> >> +
> >> + mutex_unlock(&isp->video_lock);
> >> + return ret;
> >> +}
> >> +
> >> +static unsigned int isp_video_capture_poll(struct file *file,
> >> + struct poll_table_struct *wait)
> >> +{
> >> + struct fimc_isp *isp = video_drvdata(file);
> >> + int ret;
> >> +
> >> + mutex_lock(&isp->video_lock);
> >> + ret = vb2_poll(&isp->capture_vb_queue, file, wait);
> >> + mutex_unlock(&isp->video_lock);
> >> + return ret;
> >> +}
> >> +
> >> +static int isp_video_capture_mmap(struct file *file, struct
> >> vm_area_struct *vma)
> >> +{
> >> + struct fimc_isp *isp = video_drvdata(file);
> >> + int ret;
> >> +
> >> + if (mutex_lock_interruptible(&isp->video_lock))
> >> + return -ERESTARTSYS;
> >> +
> >> + ret = vb2_mmap(&isp->capture_vb_queue, vma);
> >> + mutex_unlock(&isp->video_lock);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static const struct v4l2_file_operations isp_video_capture_fops = {
> >> + .owner = THIS_MODULE,
> >> + .open = isp_video_capture_open,
> >> + .release= isp_video_capture_close,
> >> + .poll = isp_video_capture_poll,
> >> + .unlocked_ioctl = video_ioctl2,
> >> + .mmap = isp_video_capture_mmap,
> >
> > Can't you use the helper functions vb2_fop_open/release/poll/mmap here?
>
> It seems vb2_fop_mmap/poll can be used directly, open(), release() are
> a bit more complicated as some media pipeline operations need to
> additionally be done within these callbacks. There is no vb2_fop_open(),
> and AFAICS v4l2_fh_open() is sufficient and intended as open() helper.
That's correct. Sorry for the misinformation about the non-existant
vb2_fop_open.
> For the next iteration I have used vb2_fop_release(), called indirectly,
> as it nicely simplifies things a bit.
>
> BTW, shouldn't vb2_fop_release() also be taking the lock ? Actually it is
> more useful for me in current form, but the drivers that directly assign
> it to struct v4l2_file_operations::open might be in trouble, unless I'm
> missing something.
I don't see where a lock would be needed. I don't see any concurrency here.
Nobody else can mess with the queue as long as they are not the owner.
>
> >> +};
> >> +
> >> +/*
> >> + * Video node ioctl operations
> >> + */
> [...]
> >> +static int fimc_isp_capture_streamon(struct file *file, void *priv,
> >> + enum v4l2_buf_type type)
> >> +{
> >> + struct fimc_isp *isp = video_drvdata(file);
> >> + struct v4l2_subdev *sensor = isp->pipeline.subdevs[IDX_SENSOR];
> >> + struct fimc_pipeline *p =&isp->pipeline;
> >> + int ret;
> >> +
> >> + /* TODO: check if the OTF interface is not running */
> >> +
> >> + ret = media_entity_pipeline_start(&sensor->entity, p->m_pipeline);
> >> + if (ret< 0)
> >> + return ret;
> >> +
> >> + ret = fimc_isp_pipeline_validate(isp);
> >> + if (ret) {
> >> + media_entity_pipeline_stop(&sensor->entity);
> >> + return ret;
> >> + }
> >> +
> >> + return vb2_streamon(&isp->capture_vb_queue, type);
> >> +}
> >> +
> >> +static int fimc_isp_capture_streamoff(struct file *file, void *priv,
> >> +enum v4l2_buf_type type)
> >> +{
> >> + struct fimc_isp *isp = video_drvdata(file);
> >> + struct v4l2_subdev *sd = isp->pipeline.subdevs[IDX_SENSOR];
> >> + int ret;
> >> +
> >> + ret = vb2_streamoff(&isp->capture_vb_queue, type);
> >> + if (ret == 0)
> >> + media_entity_pipeline_stop(&sd->entity);
> >> + return ret;
> >> +}
> >> +
> >> +static int fimc_isp_capture_reqbufs(struct file *file, void *priv,
> >> + struct v4l2_requestbuffers *reqbufs)
> >> +{
> >> + struct fimc_isp *isp = video_drvdata(file);
> >> + i
Re: [PATCH V4 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
Hi Viresh,
On Tue, Mar 12, 2013 at 4:19 PM, Viresh Kumar wrote:
> This is what Russell told me a long time back:
> "Don't use Adding, Fixing, etc words as this work is not something, which is
> already done."
>
> So your subject should have been: "cpufreq: exynos: Add cpufreq driver
> for exynos5440"
ok right.
>
> Fix it if you need another version, which i believe you do :)
yes no escape now :)
>
> On Tue, Mar 12, 2013 at 5:58 PM, Amit Daniel Kachhap
> wrote:
>> This patch adds dvfs support for exynos5440 SOC. This soc has 4 cores and
>> they scale at same frequency. The nature of exynos5440 clock controller is
>> different from previous exynos controllers so not using the common exynos
>> cpufreq framework. The major difference being interrupt notfication for
>
> s/notfication/notification
ok
>
>> frequency change. Also, OPP library is used for device tree parsing to get
>> different parameters like frequency, voltage etc. Since the opp library sorts
>> the frequency table in ascending order so they are again re-arranged in
>> descending order. This will have one-to-one mapping with the clock controller
>> state management logic.
>>
>> Signed-off-by: Amit Daniel Kachhap
>> ---
>> .../bindings/cpufreq/cpufreq-exynos5440.txt| 29 ++
>> drivers/cpufreq/Kconfig.arm|9 +
>> drivers/cpufreq/Makefile |1 +
>> drivers/cpufreq/exynos5440-cpufreq.c | 466
>>
>> 4 files changed, 505 insertions(+), 0 deletions(-)
>> create mode 100644
>> Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
>> create mode 100644 drivers/cpufreq/exynos5440-cpufreq.c
>>
>> diff --git
>> a/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
>> b/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
>> new file mode 100644
>> index 000..a0dbe0b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
>> @@ -0,0 +1,29 @@
>> +
>> +Exynos5440 cpufreq driver
>> +---
>> +
>> +Exynos5440 SoC cpufreq driver for CPU frequency scaling.
>> +
>> +Required properties:
>> +- interrupts: Interrupt to know the completion of cpu frequency change.
>> +- operating-points: Table of frequencies and voltage CPU could be
>> transitioned into,
>> + in the decreasing order. Frequency should be in KHZ units and voltage
>
> s/KHZ/KHz
ok
>
>> + should be in microvolts.
>
> probably s/microvolts/micro-volts ??
>
>> +
>> +Optional properties:
>> +- clock-latency: Clock monitor latency in microsecond.
>> +
>> +All the required listed above must be defined under node cpufreq.
>> +
>> +Example:
>> +
>> + cpufreq@16 {
>> + compatible = "samsung,exynos5440-cpufreq";
>> + reg = <0x16 0x1000>;
>> + interrupts = <0 57 0>;
>> + operating-points = <
>> + 100 975000
>> + 80 925000>;
>> + clock-latency = <10>;
>> + };
>> +
>
>> diff --git a/drivers/cpufreq/exynos5440-cpufreq.c
>> b/drivers/cpufreq/exynos5440-cpufreq.c
>> +static void exynos_enable_dvfs(void)
>> +{
>
>> + /* Set initial performance index */
>> + for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
>> + if (freq_table[i].frequency == dvfs_info->cur_frequency)
>> + break;
>> +
>> + if (freq_table[i].frequency == CPUFREQ_TABLE_END) {
>> + dev_crit(dvfs_info->dev, "Boot up frequency not
>> supported\n");
>> + /* Assign the highest frequency */
>> + i = 0;
>> + dvfs_info->cur_frequency = freq_table[i].frequency;
>
> What about:
>
> dvfs_info->cur_frequency = freq_table[0].frequency;
>
> as i don't see i being used again?
No It is used below for frequency setting.
>
>> + }
>
>> +}
>
>> +static int exynos_target(struct cpufreq_policy *policy,
>> + unsigned int target_freq,
>> + unsigned int relation)
>> +{
>
>> + if (cpufreq_frequency_table_target(policy, freq_table,
>> + target_freq, relation, &index)) {
>> + ret = -EINVAL;
>
> Use the error value returned by called functions, probably i gave this
> comment last time too?
yes my mistake.
>
>> + goto out;
>> + }
>
>> +}
>
>> +static void exynos_sort_descend_freq_table(void)
>> +{
>> + struct cpufreq_frequency_table *freq_tbl = dvfs_info->freq_table;
>> + int i = 0, index;
>> + unsigned int tmp_freq;
>> +
>> + /*
>> +* Freq table is already in ascending order as it is created from
>> +* OPP library, so just swap the elements to make it descending.
>
> why??
I explained this requirement in the patch commit. Will explain it here again.
>
>> +*/
>> + for (i = 0; i < dvfs_info->freq_coun
Re: [PATCH] clk: let mxs specific clk-div clock type be a generic clock type
Hello, On Mon, Mar 18, 2013 at 03:48:24PM +0530, Thomas Abraham wrote: > On 18 March 2013 14:12, Shawn Guo wrote: > > On Sat, Mar 16, 2013 at 06:20:01PM +0530, Thomas Abraham wrote: > >> The mxs platform specific clk-div clock is an extended version of the > >> basic integer divider clock type that supports checking the stability > >> status of the divider clock output. This type of clock is found on > >> some of the Samsung platforms as well. So let the mxs specfic clk-div > >> clock type be a generic clock type that all platforms can utilize. > >> > >> Cc: Shawn Guo > >> Cc: Mike Turquette > >> Signed-off-by: Thomas Abraham > >> --- > >> drivers/clk/Makefile |1 + > >> drivers/clk/clk-divider-status.c | 119 > >> ++ > >> drivers/clk/mxs/Makefile |2 +- > >> drivers/clk/mxs/clk-div.c| 110 > >> --- > >> drivers/clk/mxs/clk.h| 12 +++- > >> include/linux/clk-provider.h | 21 +++ > >> 6 files changed, 151 insertions(+), 114 deletions(-) > >> create mode 100644 drivers/clk/clk-divider-status.c > >> delete mode 100644 drivers/clk/mxs/clk-div.c > > > > From my quick testing, it seems working for mxs platform. But it's hard > > to review the changes. Making it two steps might be helpful for > > reviewer: > > > > 1) git mv drivers/clk/mxs/clk-div.c drivers/clk/clk-divider-status.c > > 2) make changes on drivers/clk/clk-divider-status.c > > Thanks Shawn for your comments. I will split this patch as you > suggested and post again. I didn't try to look at your patch, but maybe format-patch -M is enough to make the patch easier to parse (for humans). Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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] clk: let mxs specific clk-div clock type be a generic clock type
On 18 March 2013 14:12, Shawn Guo wrote:
> On Sat, Mar 16, 2013 at 06:20:01PM +0530, Thomas Abraham wrote:
>> The mxs platform specific clk-div clock is an extended version of the
>> basic integer divider clock type that supports checking the stability
>> status of the divider clock output. This type of clock is found on
>> some of the Samsung platforms as well. So let the mxs specfic clk-div
>> clock type be a generic clock type that all platforms can utilize.
>>
>> Cc: Shawn Guo
>> Cc: Mike Turquette
>> Signed-off-by: Thomas Abraham
>> ---
>> drivers/clk/Makefile |1 +
>> drivers/clk/clk-divider-status.c | 119
>> ++
>> drivers/clk/mxs/Makefile |2 +-
>> drivers/clk/mxs/clk-div.c| 110 ---
>> drivers/clk/mxs/clk.h| 12 +++-
>> include/linux/clk-provider.h | 21 +++
>> 6 files changed, 151 insertions(+), 114 deletions(-)
>> create mode 100644 drivers/clk/clk-divider-status.c
>> delete mode 100644 drivers/clk/mxs/clk-div.c
>
> From my quick testing, it seems working for mxs platform. But it's hard
> to review the changes. Making it two steps might be helpful for
> reviewer:
>
> 1) git mv drivers/clk/mxs/clk-div.c drivers/clk/clk-divider-status.c
> 2) make changes on drivers/clk/clk-divider-status.c
Thanks Shawn for your comments. I will split this patch as you
suggested and post again.
Thomas.
>
> Shawn
>
>>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index 0147022..0ac851a 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -7,6 +7,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-fixed-factor.o
>> obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o
>> obj-$(CONFIG_COMMON_CLK) += clk-gate.o
>> obj-$(CONFIG_COMMON_CLK) += clk-mux.o
>> +obj-$(CONFIG_COMMON_CLK) += clk-divider-status.o
>>
>> # SoCs specific
>> obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o
>> diff --git a/drivers/clk/clk-divider-status.c
>> b/drivers/clk/clk-divider-status.c
>> new file mode 100644
>> index 000..1d66059
>> --- /dev/null
>> +++ b/drivers/clk/clk-divider-status.c
>> @@ -0,0 +1,119 @@
>> +/*
>> + * Copyright 2012 Freescale Semiconductor, Inc.
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + *
>> + * Extension to the adjustable divider clock implementation with support for
>> + * divider clock stability checks.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +/*
>> + * DOC: Adjustable divider clock with support for divider stability check
>> + *
>> + * Traits of this clock:
>> + * prepare - clk_prepare only ensures that parents are prepared
>> + * enable - clk_enable only ensures that parents are enabled
>> + * rate - rate is adjustable. clk->rate = parent->rate / divisor
>> + * parent - fixed parent. No clk_set_parent support
>> + */
>> +
>> +static inline struct clk_divider_status *to_clk_div(struct clk_hw *hw)
>> +{
>> + struct clk_divider *divider = container_of(hw, struct clk_divider, hw);
>> +
>> + return container_of(divider, struct clk_divider_status, divider);
>> +}
>> +
>> +static unsigned long clk_divider_status_recalc_rate(struct clk_hw *hw,
>> + unsigned long parent_rate)
>> +{
>> + struct clk_divider_status *div = to_clk_div(hw);
>> +
>> + return div->ops->recalc_rate(&div->divider.hw, parent_rate);
>> +}
>> +
>> +static long clk_divider_status_round_rate(struct clk_hw *hw, unsigned long
>> rate,
>> +unsigned long *prate)
>> +{
>> + struct clk_divider_status *div = to_clk_div(hw);
>> +
>> + return div->ops->round_rate(&div->divider.hw, rate, prate);
>> +}
>> +
>> +static int clk_divider_status_set_rate(struct clk_hw *hw, unsigned long
>> rate,
>> + unsigned long parent_rate)
>> +{
>> + struct clk_divider_status *div = to_clk_div(hw);
>> + int ret;
>> +
>> + ret = div->ops->set_rate(&div->divider.hw, rate, parent_rate);
>> + if (!ret) {
>> + unsigned long timeout = jiffies + msecs_to_jiffies(10);
>> +
>> + while (readl_relaxed(div->reg) & (1 << div->busy)) {
>> + if (time_after(jiffies, timeout))
>> + return -ETIMEDOUT;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static struct clk_ops clk_divider_status_ops = {
>> + .recalc_rate = clk_divider_status_recalc_rate,
>> + .round_rate = clk_divider_status_round_rate,
>> + .set_rate = clk_divider_status_set_rate,
>> +};
>> +EXPORT_SYMBOL_GPL(clk_divider_status_ops);
>> +
>> +struct clk *clk_register_divider
Re: [PATCH] ARM: SAMSUNG: Set clock parent if provided
Hi Shaik, On 03/15/2013 10:00 AM, Shaik Ameer Basha wrote: > On Thu, Mar 7, 2013 at 9:05 PM, Sylwester Nawrocki > wrote: ... >> May I ask what do you need this for ? This code won't be used for >> Exynos4 and Exynos5 SoCs starting from 3.10. And it is going to be >> removed once other platforms are converted to the new Samsung clocks >> driver. > > I had some issues in exynos5 with out this implementation. > But yes... you are right, once we move to common clock framework (CCF) > we don't require this change, as CCF doesn't use this (for exynos 4/5). > > What about all old non-dt based platforms? > Cant they use this change until they move to DT and CCF? I have nothing against the patch, I was just curious if you need it for the new SOCs where the new clocks driver is supposed to be used. Of course other Samsung platforms could switch to CCF before having support for the device tree. Regards, Sylwester -- 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] clk: let mxs specific clk-div clock type be a generic clock type
On Sat, Mar 16, 2013 at 06:20:01PM +0530, Thomas Abraham wrote:
> The mxs platform specific clk-div clock is an extended version of the
> basic integer divider clock type that supports checking the stability
> status of the divider clock output. This type of clock is found on
> some of the Samsung platforms as well. So let the mxs specfic clk-div
> clock type be a generic clock type that all platforms can utilize.
>
> Cc: Shawn Guo
> Cc: Mike Turquette
> Signed-off-by: Thomas Abraham
> ---
> drivers/clk/Makefile |1 +
> drivers/clk/clk-divider-status.c | 119
> ++
> drivers/clk/mxs/Makefile |2 +-
> drivers/clk/mxs/clk-div.c| 110 ---
> drivers/clk/mxs/clk.h| 12 +++-
> include/linux/clk-provider.h | 21 +++
> 6 files changed, 151 insertions(+), 114 deletions(-)
> create mode 100644 drivers/clk/clk-divider-status.c
> delete mode 100644 drivers/clk/mxs/clk-div.c
>From my quick testing, it seems working for mxs platform. But it's hard
to review the changes. Making it two steps might be helpful for
reviewer:
1) git mv drivers/clk/mxs/clk-div.c drivers/clk/clk-divider-status.c
2) make changes on drivers/clk/clk-divider-status.c
Shawn
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 0147022..0ac851a 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-fixed-factor.o
> obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o
> obj-$(CONFIG_COMMON_CLK) += clk-gate.o
> obj-$(CONFIG_COMMON_CLK) += clk-mux.o
> +obj-$(CONFIG_COMMON_CLK) += clk-divider-status.o
>
> # SoCs specific
> obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o
> diff --git a/drivers/clk/clk-divider-status.c
> b/drivers/clk/clk-divider-status.c
> new file mode 100644
> index 000..1d66059
> --- /dev/null
> +++ b/drivers/clk/clk-divider-status.c
> @@ -0,0 +1,119 @@
> +/*
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + *
> + * Extension to the adjustable divider clock implementation with support for
> + * divider clock stability checks.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/*
> + * DOC: Adjustable divider clock with support for divider stability check
> + *
> + * Traits of this clock:
> + * prepare - clk_prepare only ensures that parents are prepared
> + * enable - clk_enable only ensures that parents are enabled
> + * rate - rate is adjustable. clk->rate = parent->rate / divisor
> + * parent - fixed parent. No clk_set_parent support
> + */
> +
> +static inline struct clk_divider_status *to_clk_div(struct clk_hw *hw)
> +{
> + struct clk_divider *divider = container_of(hw, struct clk_divider, hw);
> +
> + return container_of(divider, struct clk_divider_status, divider);
> +}
> +
> +static unsigned long clk_divider_status_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_divider_status *div = to_clk_div(hw);
> +
> + return div->ops->recalc_rate(&div->divider.hw, parent_rate);
> +}
> +
> +static long clk_divider_status_round_rate(struct clk_hw *hw, unsigned long
> rate,
> +unsigned long *prate)
> +{
> + struct clk_divider_status *div = to_clk_div(hw);
> +
> + return div->ops->round_rate(&div->divider.hw, rate, prate);
> +}
> +
> +static int clk_divider_status_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_divider_status *div = to_clk_div(hw);
> + int ret;
> +
> + ret = div->ops->set_rate(&div->divider.hw, rate, parent_rate);
> + if (!ret) {
> + unsigned long timeout = jiffies + msecs_to_jiffies(10);
> +
> + while (readl_relaxed(div->reg) & (1 << div->busy)) {
> + if (time_after(jiffies, timeout))
> + return -ETIMEDOUT;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static struct clk_ops clk_divider_status_ops = {
> + .recalc_rate = clk_divider_status_recalc_rate,
> + .round_rate = clk_divider_status_round_rate,
> + .set_rate = clk_divider_status_set_rate,
> +};
> +EXPORT_SYMBOL_GPL(clk_divider_status_ops);
> +
> +struct clk *clk_register_divider_status(struct device *dev, const char *name,
> + const char *parent_name, unsigned long flags, void __iomem *reg,
> + u8 shift, u8 width, u8 clk_divider_flags,
> + void __iomem *reg_status, u8 shift_status, spinlock_t *lock)
> +{
> + struct
