* [PATCH] regulator: axp20x: Fix axp809 ldo_io registration error on cold boot
From: Chen-Yu Tsai @ 2016-11-11 3:12 UTC (permalink / raw)
To: linux-arm-kernel
The maximum supported voltage for ldo_io# is 3.3V, but on cold boot
the selector comes up at 0x1f, which maps to 3.8V. This was previously
corrected by Allwinner's U-boot, which set all regulators on the PMICs
to some pre-configured voltage. With recent progress in U-boot SPL
support, this is no longer the case. In any case we should handle
this quirk in the kernel driver as well.
This invalid setting causes _regulator_get_voltage() to fail with -EINVAL
which causes regulator registration to fail when constrains are used:
[ 1.054181] vcc-pg: failed to get the current voltage(-22)
[ 1.059670] axp20x-regulator axp20x-regulator.0: Failed to register ldo_io0
[ 1.069749] axp20x-regulator: probe of axp20x-regulator.0 failed with error -22
This commits makes the axp20x regulator driver accept the 0x1f register
value, fixing this.
The datasheet does not guarantee reliable operation above 3.3V, so on
boards where this regulator is used the regulator-max-microvolt setting
must be 3.3V or less.
This is essentially the same as the commit f40d4896bf32 ("regulator:
axp20x: Fix axp22x ldo_io registration error on cold boot") for AXP22x
PMICs.
Fixes: a51f9f4622a3 ("regulator: axp20x: support AXP809 variant")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/regulator/axp20x-regulator.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 54382ef902c6..e6a512ebeae2 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -337,10 +337,18 @@ static const struct regulator_desc axp809_regulators[] = {
AXP22X_ELDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(1)),
AXP_DESC(AXP809, ELDO3, "eldo3", "eldoin", 700, 3300, 100,
AXP22X_ELDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(2)),
- AXP_DESC_IO(AXP809, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
+ /*
+ * Note the datasheet only guarantees reliable operation up to
+ * 3.3V, this needs to be enforced via dts provided constraints
+ */
+ AXP_DESC_IO(AXP809, LDO_IO0, "ldo_io0", "ips", 700, 3800, 100,
AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
- AXP_DESC_IO(AXP809, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
+ /*
+ * Note the datasheet only guarantees reliable operation up to
+ * 3.3V, this needs to be enforced via dts provided constraints
+ */
+ AXP_DESC_IO(AXP809, LDO_IO1, "ldo_io1", "ips", 700, 3800, 100,
AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
AXP_DESC_FIXED(AXP809, RTC_LDO, "rtc_ldo", "ips", 1800),
--
2.10.2
^ permalink raw reply related
* [PATCH v27 1/9] memblock: add memblock_cap_memory_range()
From: Dennis Chen @ 2016-11-11 3:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111025049.GG381@linaro.org>
On Fri, Nov 11, 2016 at 11:50:50AM +0900, AKASHI Takahiro wrote:
> Will,
> (+ Cc: Dennis)
>
> On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> > On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > > Add memblock_cap_memory_range() which will remove all the memblock regions
> > > except the range specified in the arguments.
> > >
> > > This function, like memblock_mem_limit_remove_map(), will not remove
> > > memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
> > > later as "device memory."
> > > See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
> > > address the mem limit issue").
> > >
> > > This function is used, in a succeeding patch in the series of arm64 kdump
> > > suuport, to limit the range of usable memory, System RAM, on crash dump
> > > kernel.
> > > (Please note that "mem=" parameter is of little use for this purpose.)
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Cc: linux-mm at kvack.org
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > ---
> > > include/linux/memblock.h | 1 +
> > > mm/memblock.c | 28 ++++++++++++++++++++++++++++
> > > 2 files changed, 29 insertions(+)
> > >
> > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > index 5b759c9..0e770af 100644
> > > --- a/include/linux/memblock.h
> > > +++ b/include/linux/memblock.h
> > > @@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
> > > phys_addr_t memblock_end_of_DRAM(void);
> > > void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > > void memblock_mem_limit_remove_map(phys_addr_t limit);
> > > +void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> > > bool memblock_is_memory(phys_addr_t addr);
> > > int memblock_is_map_memory(phys_addr_t addr);
> > > int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > index 7608bc3..eb53876 100644
> > > --- a/mm/memblock.c
> > > +++ b/mm/memblock.c
> > > @@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
> > > (phys_addr_t)ULLONG_MAX);
> > > }
> > >
> > > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > > +{
> > > + int start_rgn, end_rgn;
> > > + int i, ret;
> > > +
> > > + if (!size)
> > > + return;
> > > +
> > > + ret = memblock_isolate_range(&memblock.memory, base, size,
> > > + &start_rgn, &end_rgn);
> > > + if (ret)
> > > + return;
> > > +
> > > + /* remove all the MAP regions */
> > > + for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > > + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > + memblock_remove_region(&memblock.memory, i);
> > > +
> > > + for (i = start_rgn - 1; i >= 0; i--)
> > > + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > + memblock_remove_region(&memblock.memory, i);
> > > +
> > > + /* truncate the reserved regions */
> > > + memblock_remove_range(&memblock.reserved, 0, base);
> > > + memblock_remove_range(&memblock.reserved,
> > > + base + size, (phys_addr_t)ULLONG_MAX);
> > > +}
> >
> > This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> > you not implement that in terms of your new, more general, function? e.g.
> > by passing base == 0, and size == limit?
>
> Obviously it's possible.
> I actually talked to Dennis before about merging them,
> but he was against my idea.
>
Oops! I thought we have reached agreement in the thread:http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/442817.html
So feel free to do that as Will'll do
>
> Thanks,
> -Takahiro AKASHI
>
> > Will
^ permalink raw reply
* [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
From: Masahiro Yamada @ 2016-11-11 3:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110133559.GA10191@kroah.com>
2016-11-10 22:35 GMT+09:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Thu, Nov 10, 2016 at 10:24:21PM +0900, Masahiro Yamada wrote:
>>
>> sdhci_alloc_host() returns an error pointer when it fails.
>> but mmc_alloc_host() cannot.
>>
>> This series allow to propagate a proper error code
>> when host-allocation fails.
>
> Why? What can we really do about the error except give up? Why does
> having a explicit error value make any difference to the caller, they
> can't do anything different, right?
The error code is shown in the console, like
probe of 5a000000.sdhc failed with error -12
The proper error code will give a clue
why the driver failed to probe.
> I suggest just leaving it as-is, it's simple, and you don't have to mess
> with PTR_ERR() anywhere.
Why?
Most of driver just give up probing for any error,
but we still do ERR_PTR()/PTR_ERR() here and there.
I think this patch is the same pattern.
If a function returns NULL on failure, we need to think about
"what is the most common failure case".
Currently, MMC drivers assume -ENOMEM is the best
fit for mmc_alloc_host(), but the assumption is fragile.
Already, mmc_alloc_host() calls a function
that returns not only -ENOMEM, but also -ENOSPC.
In the future, some other failure cases might be
added to mmc_alloc_host().
Once we decide the API returns an error pointer,
drivers just propagate the return value from the API.
This is much more stable implementation.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH -next] cpufreq: brcmstb-avs-cpufreq: make symbol brcm_avs_cpufreq_attr static
From: Viresh Kumar @ 2016-11-11 3:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478791145-21137-1-git-send-email-weiyj.lk@gmail.com>
On 10-11-16, 15:19, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Fixes the following sparse warning:
>
> drivers/cpufreq/brcmstb-avs-cpufreq.c:982:18: warning:
> symbol 'brcm_avs_cpufreq_attr' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> drivers/cpufreq/brcmstb-avs-cpufreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/brcmstb-avs-cpufreq.c b/drivers/cpufreq/brcmstb-avs-cpufreq.c
> index b761d54..4fda623 100644
> --- a/drivers/cpufreq/brcmstb-avs-cpufreq.c
> +++ b/drivers/cpufreq/brcmstb-avs-cpufreq.c
> @@ -979,7 +979,7 @@ cpufreq_freq_attr_ro(brcm_avs_pmap);
> cpufreq_freq_attr_ro(brcm_avs_voltage);
> cpufreq_freq_attr_ro(brcm_avs_frequency);
>
> -struct freq_attr *brcm_avs_cpufreq_attr[] = {
> +static struct freq_attr *brcm_avs_cpufreq_attr[] = {
> &cpufreq_freq_attr_scaling_available_freqs,
> &brcm_avs_pstate,
> &brcm_avs_mode,
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Jisheng Zhang @ 2016-11-11 3:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <15b06a12-ed69-03a7-ccc7-0c133ce1ac1e@marvell.com>
Hi Rob, Ziji,
On Thu, 10 Nov 2016 19:44:19 +0800 Ziji Hu wrote:
> Hi Rob,
>
> On 2016/11/10 2:24, Rob Herring wrote:
> > On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
> >> From: Ziji Hu <huziji@marvell.com>
> >>
> >> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> >> Add Xenon-specific properties.
> >> Also add properties for Xenon PHY setting.
> >>
> >> Signed-off-by: Hu Ziji <huziji@marvell.com>
> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >> ---
> >> Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
> >> MAINTAINERS | 1 +-
> >> 2 files changed, 162 insertions(+), 0 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >> new file mode 100644
> >> index 000000000000..0d2d139494d3
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >> @@ -0,0 +1,161 @@
> >> +Marvell's Xenon SDHCI Controller device tree bindings
> >> +This file documents differences between the core mmc properties
> >> +described by mmc.txt and the properties used by the Xenon implementation.
> >> +
> >> +A single Xenon IP can support multiple slots.
> >> +Each slot acts as an independent SDHC. It owns independent resources, such
> >> +as register sets clock and PHY.
> >> +Each slot should have an independent device tree node.
> >> +
> >> +Required Properties:
> >> +- compatible: should be one of the following
> >> + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
> >> + Must provide a second register area and marvell,pad-type.
> >> + - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
> >> + Armada-3700.
> >
> > Need SoC specific compatible strings.
> >
>
> Xenon SDHC is a common IP for all Marvell SOCs.
> It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
> There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
> Actually only few SOCs require special properties.
> Any suggestion please?
>
> >> +
> >> +- clocks:
> >> + Array of clocks required for SDHCI.
> >> + Requires at least one for Xenon IP core.
> >> + Some SOCs require additional clock for AXI bus.
> >> +
> >> +- clock-names:
> >> + Array of names corresponding to clocks property.
> >> + The input clock for Xenon IP core should be named as "core".
> >> + The optional AXI clock should be named as "axi".
> >
> > When is AXI clock optional? This should be required for ?? compatible
> > strings.
> >
> It is required on some SOCs.
> I will double check if a suitable compatible string can be determined for those SOCs.
Besides the core clk, berlin SoCs have one AXI clock. Usually, we have two
solutions:
solA: as current patch does, take "marvell,xenon-sdhci" as compatible string
and make the AXI clock property optional. Usually for berlin SoCs, we don't need
special properties.
PS: this solution is also what sdhci-pxav3.c takes
solB: As Rob said, add extra SoC compatible strings, so we'll have
something like:
static const struct of_device_id sdhci_xenon_of_match[] = {
{ .compatible = "marvell,armada-3700-sdhci", },
{ .compatible = "marvell,berlin4ct-sdhci", },
...
{ .compatible = "marvell,berlinxxx-mmc", },
}
then we take care the AXI clk for berlin SoCs in the code.
Which solution do you prefer?
Thanks,
Jisheng
^ permalink raw reply
* [PATCH] phy: rockchip-inno-usb2: correct 480MHz output clock stable time
From: wlf @ 2016-11-11 3:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2283070.Hkeafucara@diego>
Hi Heiko?
? 2016?11?10? 17:21, Heiko St?bner ??:
> Am Donnerstag, 10. November 2016, 10:54:49 schrieb wlf:
>> Hi Doug,
>>
>> ? 2016?11?10? 04:54, Doug Anderson ??:
>>> Hi,
>>>
>>> On Mon, Nov 7, 2016 at 5:00 AM, William Wu <wulf@rock-chips.com> wrote:
>>>> We found that the system crashed due to 480MHz output clock of
>>>> USB2 PHY was unstable after clock had been enabled by gpu module.
>>>>
>>>> Theoretically, 1 millisecond is a critical value for 480MHz
>>>> output clock stable time, so we try to change the delay time
>>>> to 1.2 millisecond to avoid this issue.
>>>>
>>>> Signed-off-by: William Wu <wulf@rock-chips.com>
>>>> ---
>>>>
>>>> drivers/phy/phy-rockchip-inno-usb2.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c
>>>> b/drivers/phy/phy-rockchip-inno-usb2.c index ecfd7d1..8f2d2b6 100644
>>>> --- a/drivers/phy/phy-rockchip-inno-usb2.c
>>>> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
>>>> @@ -267,7 +267,7 @@ static int rockchip_usb2phy_clk480m_enable(struct
>>>> clk_hw *hw)>>
>>>> return ret;
>>>>
>>>> /* waitting for the clk become stable */
>>>>
>>>> - mdelay(1);
>>>> + udelay(1200);
>>> Several people who have seen this patch have expressed concern that a
>>> 1.2 ms delay is pretty long for something that's supposed to be
>>> "atomic" like a clk_enable(). Consider that someone might call
>>> clk_enable() while interrupts are disabled and that a 1.2 ms interrupt
>>> latency is not so great.
>>>
>>> It seems like this clock should be moved to be enabled in "prepare"
>>> and the "enable" should be a no-op. This is a functionality change,
>>> but I don't think there are any real users for this clock at the
>>> moment so it should be fine.
>>>
>>> (of course, the 1 ms latency that existed before this patch was still
>>> pretty bad, but ...)
>> Thanks a lot for your suggestion.
>> I agree with you. clk_enable() will call spin_lock_irqsave() to disable
>> interrupt, and we add
>> more than 1ms in clk_enable may cause big latency.
>>
>> And according to clk_prepare() description:
>> In a simple case, clk_prepare can be used instead of clk_enable to
>> ungate a clk if the
>> operation may sleep. One example is a clk which is accessed over I2c.
>>
>> So maybe we can remove the clock to clk_prepare.
>>
>> Hi Heiko, Frank,
>> What do you think of it?
> moving to clk_prepare sounds sensible. That way you can switch from delay to
> sleep functions as well.
Thanks very much.
I will try to update a new patch.
Best regards,
Wulf
>
>
> Heiko
>
>
>
^ permalink raw reply
* [PATCH 0/2] mfd: axp20x: Fix AXP806 access errors on cold boot
From: Chen-Yu Tsai @ 2016-11-11 3:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lee,
Recently we've added full SPL support for A80 to mainline U-boot. This
means we no longer depend on Allwinner's bootloader. It also means that
some of system configuration the bootloader set up no longer applies.
The bootloader was correctly configuring the multi-device addressing
support in the AXP806 PMIC. If the PMIC is not correctly addressed, it
just ignores all reads and writes to the other registers. As mainline
U-boot does not support the AXP806, and since we can't always count on
a good bootloader, we should re-configure this in the kernel regardless.
Patch 1 adds the registers for the multi-device addressing scheme to
the AXP806 regmap.
Patch 2 configures the register at probe time, and reinitializes the
regmap cache.
Since support for this PMIC was just added in 4.9-rc1, I hope you can
merge these 2 patches as fixes for 4.9.
Thank you!
Regards
ChenYu
Chen-Yu Tsai (2):
mfd: axp20x: Add address extension registers for AXP806 regmap
mfd: axp20x: Fix AXP806 access errors on cold boot
drivers/mfd/axp20x.c | 38 +++++++++++++++++++++++++++++++++++++-
include/linux/mfd/axp20x.h | 2 ++
2 files changed, 39 insertions(+), 1 deletion(-)
--
2.10.2
^ permalink raw reply
* [PATCH 1/2] mfd: axp20x: Add address extension registers for AXP806 regmap
From: Chen-Yu Tsai @ 2016-11-11 3:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111032953.20849-1-wens@csie.org>
The AXP806 supports either master/standalone or slave mode.
Slave mode allows sharing the serial bus, even with multiple
AXP806 which all have the same hardware address.
This is done with extra "serial interface address extension",
or AXP806_BUS_ADDR_EXT, and "register address extension", or
AXP806_REG_ADDR_EXT, registers. The former is read-only, with
1 bit customizable at the factory, and 1 bit depending on the
state of an external pin. The latter is writable. Only when
the these device addressing bits (in the upper 4 bits of the
registers) match, will the device respond to operations on
its other registers.
Add these 2 registers to the regmap so we can access them.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/mfd/axp20x.c | 3 ++-
include/linux/mfd/axp20x.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index ba130be32e61..cdaeb34a9a38 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -135,6 +135,7 @@ static const struct regmap_range axp806_writeable_ranges[] = {
regmap_reg_range(AXP806_PWR_OUT_CTRL1, AXP806_CLDO3_V_CTRL),
regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ2_EN),
regmap_reg_range(AXP20X_IRQ1_STATE, AXP20X_IRQ2_STATE),
+ regmap_reg_range(AXP806_REG_ADDR_EXT, AXP806_REG_ADDR_EXT),
};
static const struct regmap_range axp806_volatile_ranges[] = {
@@ -305,7 +306,7 @@ static const struct regmap_config axp806_regmap_config = {
.val_bits = 8,
.wr_table = &axp806_writeable_table,
.volatile_table = &axp806_volatile_table,
- .max_register = AXP806_VREF_TEMP_WARN_L,
+ .max_register = AXP806_REG_ADDR_EXT,
.cache_type = REGCACHE_RBTREE,
};
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index fec597fb34cb..7e85ececcedf 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -115,6 +115,8 @@ enum {
#define AXP806_CLDO2_V_CTRL 0x25
#define AXP806_CLDO3_V_CTRL 0x26
#define AXP806_VREF_TEMP_WARN_L 0xf3
+#define AXP806_BUS_ADDR_EXT 0xfe
+#define AXP806_REG_ADDR_EXT 0xff
/* Interrupt */
#define AXP152_IRQ1_EN 0x40
--
2.10.2
^ permalink raw reply related
* [PATCH 2/2] mfd: axp20x: Fix AXP806 access errors on cold boot
From: Chen-Yu Tsai @ 2016-11-11 3:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111032953.20849-1-wens@csie.org>
The AXP806 supports either master/standalone or slave mode.
Slave mode allows sharing the serial bus, even with multiple
AXP806 which all have the same hardware address.
This is done with extra "serial interface address extension",
or AXP806_BUS_ADDR_EXT, and "register address extension", or
AXP806_REG_ADDR_EXT, registers. The former is read-only, with
1 bit customizable at the factory, and 1 bit depending on the
state of an external pin. The latter is writable. Only when
the these device addressing bits (in the upper 4 bits of the
registers) match, will the device respond to operations on
its other registers.
The AXP806_REG_ADDR_EXT was previously configured by Allwinner's
bootloader. Work on U-boot SPL support now allows us to switch
to mainline U-boot, which doesn't do this for us. There might
be other bare minimum bootloaders out there which don't to this
either. It's best to handle this in the kernel.
This patch sets AXP806_REG_ADDR_EXT to 0x10, which is what we
know to be the proper value for a standard AXP806 in slave mode.
Afterwards it will reinitialize the regmap cache, to purge any
invalid stale values.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/mfd/axp20x.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index cdaeb34a9a38..2f5d46f28511 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -829,6 +829,41 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
{
int ret;
+ /*
+ * The AXP806 supports either master/standalone or slave mode.
+ * Slave mode allows sharing the serial bus, even with multiple
+ * AXP806 which all have the same hardware address.
+ *
+ * This is done with extra "serial interface address extension",
+ * or AXP806_BUS_ADDR_EXT, and "register address extension", or
+ * AXP806_REG_ADDR_EXT, registers. The former is read-only, with
+ * 1 bit customizable at the factory, and 1 bit depending on the
+ * state of an external pin. The latter is writable. Only when
+ * the these device addressing bits (in the upper 4 bits of the
+ * registers) match, will the device respond to operations on its
+ * other registers.
+ *
+ * Since we only support an AXP806 chained to an AXP809 in slave
+ * mode, and there isn't any existing hardware which uses AXP806
+ * in master mode, or has 2 AXP806s in the same system, we can
+ * just program the register address extension to the slave mode
+ * address.
+ */
+ if (axp20x->variant == AXP806_ID) {
+ /* Write to the register address extension register */
+ regmap_write(axp20x->regmap, AXP806_REG_ADDR_EXT, 0x10);
+
+ /* Make sure the write hits the device */
+ regcache_sync_region(axp20x->regmap, AXP806_REG_ADDR_EXT,
+ AXP806_REG_ADDR_EXT);
+
+ /*
+ * Reinitialize the regmap cache in case the device didn't
+ * properly respond to our reads before.
+ */
+ regmap_reinit_cache(axp20x->regmap, axp20x->regmap_cfg);
+ }
+
ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
IRQF_ONESHOT | IRQF_SHARED, -1,
axp20x->regmap_irq_chip,
--
2.10.2
^ permalink raw reply related
* [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
From: John Syne @ 2016-11-11 3:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b7232421-10f7-2bac-0ad8-847804126bf2@ti.com>
> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
>
> Hi,
>
> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>> OK, then back to my original question. Given that these DT properties are supported in the driver
>>
>
> Below properties are supported by only by ti_am3335x_adc driver and not
> ti_am335x_tsc driver. As author of this patch pointed out in another
> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
> see a use case where these values needs to be tweaked for tsc channels,
> therefore it does not make sense to be DT properties.
Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.
tscadc: tscadc at 44e0d000 {
compatible = "ti,am3359-tscadc";
reg = <0x44e0d000 0x1000>;
interrupt-parent = <&intc>;
interrupts = <16>;
ti,hwmods = "adc_tsc";
status = "disabled";
dmas = <&edma 53 0>, <&edma 57 0>;
dma-names = "fifo0", "fifo1?;
tsc {
compatible = "ti,am3359-tsc";
};
am335x_adc: adc {
#io-channel-cells = <1>;
ti,adc-channels = <0 1 2 3 4 5 6>;
ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
compatible = "ti,am3359-adc";
};
};
[1]https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
Regards,
John
>
>> shouldn?t the following be added to am33xx.dtsi and am4372.dtsi?
>
> Its totally upto board dts files to allocate channels for tsc and adc.
> So, how could these be added to dtsi files?
>
>> ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>> ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>> ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>
>> Regards,
>> John
>>>
>>> --
>>> Regards
>>> Vignesh
>>
>
> --
> Regards
> Vignesh
^ permalink raw reply
* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Jisheng Zhang @ 2016-11-11 3:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111112243.7431625d@xhacker>
On Fri, 11 Nov 2016 11:22:43 +0800 Jisheng Zhang wrote:
> Hi Rob, Ziji,
>
> On Thu, 10 Nov 2016 19:44:19 +0800 Ziji Hu wrote:
>
> > Hi Rob,
> >
> > On 2016/11/10 2:24, Rob Herring wrote:
> > > On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
> > >> From: Ziji Hu <huziji@marvell.com>
> > >>
> > >> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> > >> Add Xenon-specific properties.
> > >> Also add properties for Xenon PHY setting.
> > >>
> > >> Signed-off-by: Hu Ziji <huziji@marvell.com>
> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > >> ---
> > >> Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
> > >> MAINTAINERS | 1 +-
> > >> 2 files changed, 162 insertions(+), 0 deletions(-)
> > >> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >> new file mode 100644
> > >> index 000000000000..0d2d139494d3
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >> @@ -0,0 +1,161 @@
> > >> +Marvell's Xenon SDHCI Controller device tree bindings
> > >> +This file documents differences between the core mmc properties
> > >> +described by mmc.txt and the properties used by the Xenon implementation.
> > >> +
> > >> +A single Xenon IP can support multiple slots.
> > >> +Each slot acts as an independent SDHC. It owns independent resources, such
> > >> +as register sets clock and PHY.
> > >> +Each slot should have an independent device tree node.
> > >> +
> > >> +Required Properties:
> > >> +- compatible: should be one of the following
> > >> + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
> > >> + Must provide a second register area and marvell,pad-type.
> > >> + - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
> > >> + Armada-3700.
> > >
> > > Need SoC specific compatible strings.
> > >
> >
> > Xenon SDHC is a common IP for all Marvell SOCs.
> > It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
> > There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
> > Actually only few SOCs require special properties.
> > Any suggestion please?
> >
> > >> +
> > >> +- clocks:
> > >> + Array of clocks required for SDHCI.
> > >> + Requires at least one for Xenon IP core.
> > >> + Some SOCs require additional clock for AXI bus.
> > >> +
> > >> +- clock-names:
> > >> + Array of names corresponding to clocks property.
> > >> + The input clock for Xenon IP core should be named as "core".
> > >> + The optional AXI clock should be named as "axi".
> > >
> > > When is AXI clock optional? This should be required for ?? compatible
> > > strings.
> > >
> > It is required on some SOCs.
> > I will double check if a suitable compatible string can be determined for those SOCs.
>
> Besides the core clk, berlin SoCs have one AXI clock. Usually, we have two
> solutions:
>
> solA: as current patch does, take "marvell,xenon-sdhci" as compatible string
> and make the AXI clock property optional. Usually for berlin SoCs, we don't need
> special properties.
Personally, I prefer solA: use the IP name as compatible string. This is IP
specific rather than SoC specific. The HW itself supports two clks
Thanks,
Jisheng
>
> PS: this solution is also what sdhci-pxav3.c takes
>
> solB: As Rob said, add extra SoC compatible strings, so we'll have
> something like:
>
> static const struct of_device_id sdhci_xenon_of_match[] = {
> { .compatible = "marvell,armada-3700-sdhci", },
> { .compatible = "marvell,berlin4ct-sdhci", },
> ...
> { .compatible = "marvell,berlinxxx-mmc", },
> }
>
> then we take care the AXI clk for berlin SoCs in the code.
>
>
> Which solution do you prefer?
>
> Thanks,
> Jisheng
^ permalink raw reply
* No subject
From: Chunyan Zhang @ 2016-11-11 3:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi Steven,
On 21 October 2016 at 20:13, Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
> On 18 October 2016 at 23:44, Steven Rostedt <rostedt@goodmis.org> wrote:
>> On Tue, 18 Oct 2016 16:08:58 +0800
>> Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>>
>>> Currently Function traces can be only exported to ring buffer, this
>>> patch added trace_export concept which can process traces and export
>>> them to a registered destination as an addition to the current only
>>> one output of Ftrace - i.e. ring buffer.
>>>
>>> In this way, if we want Function traces to be sent to other destination
>>> rather than ring buffer only, we just need to register a new trace_export
>>> and implement its own .write() function for writing traces to storage.
>>>
>>> With this patch, only Function trace (trace type is TRACE_FN)
>>> is supported.
>>
>> This is getting better, but I still have some nits.
>>
>
> Thanks.
>
>>>
>>> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> ---
>>> include/linux/trace.h | 28 +++++++++++
>>> kernel/trace/trace.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>> 2 files changed, 159 insertions(+), 1 deletion(-)
>>> create mode 100644 include/linux/trace.h
>>>
>>> diff --git a/include/linux/trace.h b/include/linux/trace.h
>>> new file mode 100644
>>> index 0000000..eb1c5b8
>>> --- /dev/null
>>> +++ b/include/linux/trace.h
>>> @@ -0,0 +1,28 @@
>>> +#ifndef _LINUX_TRACE_H
>>> +#define _LINUX_TRACE_H
>>> +
>>> +#ifdef CONFIG_TRACING
>>> +/*
>>> + * The trace export - an export of Ftrace output. The trace_export
>>> + * can process traces and export them to a registered destination as
>>> + * an addition to the current only output of Ftrace - i.e. ring buffer.
>>> + *
>>> + * If you want traces to be sent to some other place rather than ring
>>> + * buffer only, just need to register a new trace_export and implement
>>> + * its own .write() function for writing traces to the storage.
>>> + *
>>> + * next - pointer to the next trace_export
>>> + * write - copy traces which have been delt with ->commit() to
>>> + * the destination
>>> + */
>>> +struct trace_export {
>>> + struct trace_export __rcu *next;
>>> + void (*write)(const char *, unsigned int);
>>
>> Why const char*? Why not const void *? This will never be a string.
>>
>
> Will revise this.
>
>>
>>> +};
>>> +
>>> +int register_ftrace_export(struct trace_export *export);
>>> +int unregister_ftrace_export(struct trace_export *export);
>>> +
>>> +#endif /* CONFIG_TRACING */
>>> +
>>> +#endif /* _LINUX_TRACE_H */
>>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>>> index 8696ce6..db94ec1 100644
>>> --- a/kernel/trace/trace.c
>>> +++ b/kernel/trace/trace.c
>>> @@ -40,6 +40,7 @@
>>> #include <linux/poll.h>
>>> #include <linux/nmi.h>
>>> #include <linux/fs.h>
>>> +#include <linux/trace.h>
>>> #include <linux/sched/rt.h>
>>>
>>> #include "trace.h"
>>> @@ -2128,6 +2129,132 @@ void trace_buffer_unlock_commit_regs(struct trace_array *tr,
>>> ftrace_trace_userstack(buffer, flags, pc);
>>> }
>>>
>>> +static void
>>> +trace_process_export(struct trace_export *export,
>>> + struct ring_buffer_event *event)
>>> +{
>>> + struct trace_entry *entry;
>>> + unsigned int size = 0;
>>> +
>>> + entry = ring_buffer_event_data(event);
>>> +
>>> + size = ring_buffer_event_length(event);
>>> +
>>> + if (export->write)
>>> + export->write((char *)entry, size);
>>
>> Is there ever going to be a time where export->write wont be set?
>
> There hasn't been since only one trace_export (i.e. stm_ftrace) was
> added in this patch-set , I just wanted to make sure the write() has
> been set before registering trace_export like what I added in 2/3 of
> this series.
>
>>
>> And if there is, this can be racy. As in
>>
>>
>> CPU 0: CPU 1:
>> ------ ------
>> if (export->write)
>>
>> export->write = NULL;
>
> Is there going to be this kind of use case? Why some one needs to
> change export->write() rather than register a new trace_export?
>
> I probably haven't understood your point thoroughly, please correct me
> if my guess was wrong.
>
Any further comments? :)
Thanks,
Chunyan
>
> Thanks for the review,
> Chunyan
>
>>
>> export->write(entry, size);
>>
>> BOOM!
>>
>>
>> -- Steve
>>
>>> +}
>>> +
>>> +static DEFINE_MUTEX(ftrace_export_lock);
>>> +
>>> +static struct trace_export __rcu *ftrace_exports_list __read_mostly;
>>> +
>>> +static DEFINE_STATIC_KEY_FALSE(ftrace_exports_enabled);
>>> +
>>> +static inline void ftrace_exports_enable(void)
>>> +{
>>> + static_branch_enable(&ftrace_exports_enabled);
>>> +}
>>> +
>>> +static inline void ftrace_exports_disable(void)
>>> +{
>>> + static_branch_disable(&ftrace_exports_enabled);
>>> +}
>>> +
>>> +void ftrace_exports(struct ring_buffer_event *event)
>>> +{
>>> + struct trace_export *export;
>>> +
>>> + preempt_disable_notrace();
>>> +
>>> + export = rcu_dereference_raw_notrace(ftrace_exports_list);
>>> + while (export) {
>>> + trace_process_export(export, event);
>>> + export = rcu_dereference_raw_notrace(export->next);
>>> + }
>>> +
>>> + preempt_enable_notrace();
>>> +}
>>> +
>>> +static inline void
>>> +add_trace_export(struct trace_export **list, struct trace_export *export)
>>> +{
>>> + rcu_assign_pointer(export->next, *list);
>>> + /*
>>> + * We are entering export into the list but another
>>> + * CPU might be walking that list. We need to make sure
>>> + * the export->next pointer is valid before another CPU sees
>>> + * the export pointer included into the list.
>>> + */
>>> + rcu_assign_pointer(*list, export);
>>> +}
>>> +
>>> +static inline int
>>> +rm_trace_export(struct trace_export **list, struct trace_export *export)
>>> +{
>>> + struct trace_export **p;
>>> +
>>> + for (p = list; *p != NULL; p = &(*p)->next)
>>> + if (*p == export)
>>> + break;
>>> +
>>> + if (*p != export)
>>> + return -1;
>>> +
>>> + rcu_assign_pointer(*p, (*p)->next);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static inline void
>>> +add_ftrace_export(struct trace_export **list, struct trace_export *export)
>>> +{
>>> + if (*list == NULL)
>>> + ftrace_exports_enable();
>>> +
>>> + add_trace_export(list, export);
>>> +}
>>> +
>>> +static inline int
>>> +rm_ftrace_export(struct trace_export **list, struct trace_export *export)
>>> +{
>>> + int ret;
>>> +
>>> + ret = rm_trace_export(list, export);
>>> + if (*list == NULL)
>>> + ftrace_exports_disable();
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +int register_ftrace_export(struct trace_export *export)
>>> +{
>>> + if (WARN_ON_ONCE(!export->write))
>>> + return -1;
>>> +
>>> + mutex_lock(&ftrace_export_lock);
>>> +
>>> + add_ftrace_export(&ftrace_exports_list, export);
>>> +
>>> + mutex_unlock(&ftrace_export_lock);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(register_ftrace_export);
>>> +
>>> +int unregister_ftrace_export(struct trace_export *export)
>>> +{
>>> + int ret;
>>> +
>>> + mutex_lock(&ftrace_export_lock);
>>> +
>>> + ret = rm_ftrace_export(&ftrace_exports_list, export);
>>> +
>>> + mutex_unlock(&ftrace_export_lock);
>>> +
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(unregister_ftrace_export);
>>> +
>>> void
>>> trace_function(struct trace_array *tr,
>>> unsigned long ip, unsigned long parent_ip, unsigned long flags,
>>> @@ -2146,8 +2273,11 @@ trace_function(struct trace_array *tr,
>>> entry->ip = ip;
>>> entry->parent_ip = parent_ip;
>>>
>>> - if (!call_filter_check_discard(call, entry, buffer, event))
>>> + if (!call_filter_check_discard(call, entry, buffer, event)) {
>>> + if (static_branch_unlikely(&ftrace_exports_enabled))
>>> + ftrace_exports(event);
>>> __buffer_unlock_commit(buffer, event);
>>> + }
>>> }
>>>
>>> #ifdef CONFIG_STACKTRACE
>>
^ permalink raw reply
* [PATCH v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
From: Javier Martinez Canillas @ 2016-11-11 4:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v66H1doBOdPPN9kz1vdNYx8gpgkF6s3UhTiXCCp0RZYKwg@mail.gmail.com>
Hello ChenYu
On Fri, Nov 11, 2016 at 12:01 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Fri, Nov 11, 2016 at 5:42 AM, Rob Herring <robh@kernel.org> wrote:
[snip]
>>
>> Do you have patches using this already. If not, it is starting to get
>> a bit late for v4.10.
>>
>> I can apply this, but then you'll just be pulling in other DT patches.
>
> Not sure what you mean by "using" this...
>
> I have patches which use this to add DT-based modalias entries for
> module auto-loading to i2c and sunxi-rsb that I haven't sent.
>
Unfortunately the I2C core can't be changed without breaking a lot of
I2C drivers that are relying on the current behavior. I've already
posted a RFC patch [0] for I2C that does this about a year ago and
enumerated the issues that have to be addressed before the change can
be made (and fixed some of the issues mentioned) on this series [1].
Another issue is that an I2C device ID table is a requirement anyways
since I2C drivers expect an i2c_device_id as an argument of their
probe function. Kieran already have patches [2] to change that which
should land soon.
I plan to fix the remaining I2C drivers once his patches are merged
and re-post the RFC patch as a proper one.
> As far as DT usage goes, we already need this for the axp20x mfd driver.
> There are 2 variants, i2c and sunxi-rsb. For the I2C variant a fix was
> sent to fix module auto-loading by using the I2C client ID table:
>
> mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
> https://git.kernel.org/cgit/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=b7142a19321484bd7681aa547c1d50148c8e2825
>
Yes, this is the workaround used by most DT-only I2C drivers. The only
reason that these drivers have an I2C device ID is due the
restrictions imposed by the I2C core.
Best regards,
Javier
^ permalink raw reply
* [PATCH v2 1/3] ARM: dts: imx6qdl-apalis: Do not rely on DDC I2C bus bitbang for HDMI
From: Stefan Agner @ 2016-11-11 4:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7eb3df71-6d1a-006a-cc4c-83ff8df3c42c@mentor.com>
On 2016-11-10 10:29, Vladimir Zapolskiy wrote:
> Hi Stefan, Philipp,
>
> On 11/09/2016 02:50 AM, Stefan Agner wrote:
>> On 2016-11-08 09:33, maitysanchayan at gmail.com wrote:
>>> Hello Shawn,
>>>
>>> On 16-10-22 15:43:04, Vladimir Zapolskiy wrote:
>>>> Hi Shawn,
>>>>
>>>> On 10/22/2016 06:25 AM, Shawn Guo wrote:
>>>>> On Mon, Sep 19, 2016 at 10:41:51AM +0530, Sanchayan Maity wrote:
>>>>>> Remove the use of DDC I2C bus bitbang to support reading of EDID
>>>>>> and rely on support from internal HDMI I2C master controller instead.
>>>>>> As a result remove the device tree property ddc-i2c-bus.
>>>>>>
>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>
>>>>> I think that the dw-hdmi i2c support [1] is a prerequisite of this
>>>>> patch. I do not see it lands on v4.9-rc1. Or am I missing something?
>>>>>
>>>>> Shawn
>>>>>
>>>>> [1] https://patchwork.kernel.org/patch/9296883/
>>>>>
>>>>
>>>> I'm adding Philipp to Cc, since he is the last one who tested the change
>>>> and helped me to push the change to the mainline:
>>>>
>>>> https://lists.freedesktop.org/archives/dri-devel/2016-September/118569.html
>>>>
>>>> The problem is that there is no official DW HDMI bridge maintainer, may be
>>>> you can review the change, and if you find it satisfactory push it through
>>>> ARM/iMX tree.
>>>
>>> Shawn, is it okay if that patch goes through your ARM/iMX tree?
>>
>> I don't think it makes sense that the DRM bridge changes go through
>> Shawn's tree. Dave should merge Philipps pull request...
>>
>
> Philipp, do you mind to submit a rebased pull request one more time?
>
You probably saw it, Dave merged it today in his drm-next branch:
https://cgit.freedesktop.org/~airlied/linux/?h=drm-next
I guess with that Shawn can go ahead and merge this patchset for next
too...?
--
Stefan
^ permalink raw reply
* [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Jassi Brar @ 2016-11-11 5:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478776558.15447.2.camel@mtksdaap41>
On Thu, Nov 10, 2016 at 4:45 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
>> Hi,
>>
>> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
>> to help write registers with critical time limitation, such as
>> updating display configuration during the vblank. It controls Global
>> Command Engine (GCE) hardware to achieve this requirement.
>>
>> These patches have a build dependency on top of v4.9-rc1.
>>
>> Changes since v15:
>> - separate "suspend and resume" patch from "save energy" patch
>> - don't stop running tasks in cmdq_suspend()
>> (i.e. leave no running tasks guarantee to clients)
>>
>> Best regards,
>> HS Liao
>>
>> HS Liao (5):
>> dt-bindings: soc: Add documentation for the MediaTek GCE unit
>> CMDQ: Mediatek CMDQ driver
>> arm64: dts: mt8173: Add GCE node
>> CMDQ: suspend and resume
>> CMDQ: save energy
>>
>> .../devicetree/bindings/mailbox/mtk-gce.txt | 43 ++
>> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 +
>> drivers/mailbox/Kconfig | 10 +
>> drivers/mailbox/Makefile | 2 +
>> drivers/mailbox/mtk-cmdq-mailbox.c | 632 +++++++++++++++++++++
>> drivers/soc/mediatek/Kconfig | 11 +
>> drivers/soc/mediatek/Makefile | 1 +
>> drivers/soc/mediatek/mtk-cmdq-helper.c | 310 ++++++++++
>> include/linux/mailbox/mtk-cmdq-mailbox.h | 67 +++
>> include/linux/soc/mediatek/mtk-cmdq.h | 182 ++++++
>> 10 files changed, 1268 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
>> create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
>> create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
>> create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
>> create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
>>
>
>
> Hi Jassi, Matthias,
>
> Sorry to disturb you.
>
No, you don't disturb, but the controller driver and protocol driver,
introduced in the same patch, does :) So does the suspend/resume
support (patch 4&5) added separately as a patch on top. Please
reorganise the patchset.
Thanks.
^ permalink raw reply
* [v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Archit Taneja @ 2016-11-11 6:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFqH_52A4=GubRjPa8X1=STExXFZy2HdoASEGUBYwqGt0BEFcw@mail.gmail.com>
Hi Jitao,
I couldn't locate the original mail, so posting on this thread instead.
Some comments below.
On 11/10/2016 10:09 PM, Enric Balletbo Serra wrote:
> Hi Jitao,
>
> 2016-08-27 8:44 GMT+02:00 Jitao Shi <jitao.shi@mediatek.com>:
>> This patch adds drm_bridge driver for parade DSI to eDP bridge chip.
>>
>> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
>> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
>> ---
>> Changes since v16:
>> - Disable ps8640 DSI MCS Function.
>> - Rename gpios name more clearly.
>> - Tune the ps8640 power on sequence.
>>
>> Changes since v15:
>> - Drop drm_connector_(un)register calls from parade ps8640.
>> The main DRM driver mtk_drm_drv now calls
>> drm_connector_register_all() after drm_dev_register() in the
>> mtk_drm_bind() function. That function should iterate over all
>> connectors and call drm_connector_register() for each of them.
>> So, remove drm_connector_(un)register calls from parade ps8640.
>>
>> Changes since v14:
>> - update copyright info.
>> - change bridge_to_ps8640 and connector_to_ps8640 to inline function.
>> - fix some coding style.
>> - use sizeof as array counter.
>> - use drm_get_edid when read edid.
>> - add mutex when firmware updating.
>>
>> Changes since v13:
>> - add const on data, ps8640_write_bytes(struct i2c_client *client, const u8 *data, u16 data_len)
>> - fix PAGE2_SW_REST tyro.
>> - move the buf[3] init to entrance of the function.
>>
>> Changes since v12:
>> - fix hw_chip_id build warning
>>
>> Changes since v11:
>> - Remove depends on I2C, add DRM depends
>> - Reuse ps8640_write_bytes() in ps8640_write_byte()
>> - Use timer check for polling like the routines in <linux/iopoll.h>
>> - Fix no drm_connector_unregister/drm_connector_cleanup when ps8640_bridge_attach fail
>> - Check the ps8640 hardware id in ps8640_validate_firmware
>> - Remove fw_version check
>> - Move ps8640_validate_firmware before ps8640_enter_bl
>> - Add ddc_i2c unregister when probe fail and ps8640_remove
>> ---
>> drivers/gpu/drm/bridge/Kconfig | 12 +
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/parade-ps8640.c | 1077 ++++++++++++++++++++++++++++++++
>> 3 files changed, 1090 insertions(+)
>> create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index b590e67..c59d043 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -50,6 +50,18 @@ config DRM_PARADE_PS8622
>> ---help---
>> Parade eDP-LVDS bridge chip driver.
>>
>> +config DRM_PARADE_PS8640
>> + tristate "Parade PS8640 MIPI DSI to eDP Converter"
>> + depends on DRM
>> + depends on OF
>> + select DRM_KMS_HELPER
>> + select DRM_MIPI_DSI
>> + select DRM_PANEL
>> + ---help---
>> + Choose this option if you have PS8640 for display
>> + The PS8640 is a high-performance and low-power
>> + MIPI DSI to eDP converter
>> +
>> config DRM_SII902X
>> tristate "Silicon Image sii902x RGB/HDMI bridge"
>> depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index efdb07e..3360537 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>> obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>> +obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
>> obj-$(CONFIG_DRM_SII902X) += sii902x.o
>> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
>> new file mode 100644
>> index 0000000..7d67431
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
>> @@ -0,0 +1,1077 @@
>> +/*
>> + * Copyright (c) 2016 MediaTek Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/firmware.h>
>> +#include <linux/gpio.h>
Not needed.
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>
The above 2 aren't needed.
>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <asm/unaligned.h>
>> +#include <drm/drm_panel.h>
>> +
>> +#include <drmP.h>
>> +#include <drm_atomic_helper.h>
>> +#include <drm_crtc_helper.h>
>> +#include <drm_crtc.h>
Not needed.
>> +#include <drm_edid.h>
>> +#include <drm_mipi_dsi.h>
>> +
>> +#define PAGE1_VSTART 0x6b
>> +#define PAGE2_SPI_CFG3 0x82
>> +#define I2C_TO_SPI_RESET 0x20
>> +#define PAGE2_ROMADD_BYTE1 0x8e
>> +#define PAGE2_ROMADD_BYTE2 0x8f
>> +#define PAGE2_SWSPI_WDATA 0x90
>> +#define PAGE2_SWSPI_RDATA 0x91
>> +#define PAGE2_SWSPI_LEN 0x92
>> +#define PAGE2_SWSPI_CTL 0x93
>> +#define TRIGGER_NO_READBACK 0x05
>> +#define TRIGGER_READBACK 0x01
>> +#define PAGE2_SPI_STATUS 0x9e
>> +#define SPI_READY 0x0c
>> +#define PAGE2_GPIO_L 0xa6
>> +#define PAGE2_GPIO_H 0xa7
>> +#define PS_GPIO9 BIT(1)
>> +#define PAGE2_IROM_CTRL 0xb0
>> +#define IROM_ENABLE 0xc0
>> +#define IROM_DISABLE 0x80
>> +#define PAGE2_SW_RESET 0xbc
>> +#define SPI_SW_RESET BIT(7)
>> +#define MPU_SW_RESET BIT(6)
>> +#define PAGE2_ENCTLSPI_WR 0xda
>> +#define PAGE2_I2C_BYPASS 0xea
>> +#define I2C_BYPASS_EN 0xd0
>> +#define PAGE2_MCS_EN 0xf3
>> +#define MCS_EN BIT(0)
>> +#define PAGE3_SET_ADD 0xfe
>> +#define PAGE3_SET_VAL 0xff
>> +#define VDO_CTL_ADD 0x13
>> +#define VDO_DIS 0x18
>> +#define VDO_EN 0x1c
>> +#define PAGE4_REV_L 0xf0
>> +#define PAGE4_REV_H 0xf1
>> +#define PAGE4_CHIP_L 0xf2
>> +#define PAGE4_CHIP_H 0xf3
>> +
>> +/* Firmware */
>> +#define PS_FW_NAME "ps864x_fw.bin"
>> +
>
> About the firmware discussion I think that if you want to maintain the
> upgrade firmware thing you should also include this patch in the
> series.
>
> https://chromium-review.googlesource.com/#/c/317221/
>
> Otherwise, if this is not really needed I think that remove this from
> the driver is the best. Just an opinion, this is something the
> maintainer should decide.
Was there a conclusion on this? As Daniel Kurtz suggested, can we drop
the update firmware stuff for now and try to get the functional part
for 4.10?
>
>> +#define FW_CHIP_ID_OFFSET 0
>> +#define FW_VERSION_OFFSET 2
>> +#define EDID_I2C_ADDR 0x50
>> +
>> +#define WRITE_STATUS_REG_CMD 0x01
>> +#define READ_STATUS_REG_CMD 0x05
>> +#define BUSY BIT(0)
>> +#define CLEAR_ALL_PROTECT 0x00
>> +#define BLK_PROTECT_BITS 0x0c
>> +#define STATUS_REG_PROTECT BIT(7)
>> +#define WRITE_ENABLE_CMD 0x06
>> +#define CHIP_ERASE_CMD 0xc7
>> +#define MAX_DEVS 0x8
>> +
>> +struct ps8640_info {
>> + u8 family_id;
>> + u8 variant_id;
>> + u16 version;
>> +};
>> +
>> +struct ps8640 {
>> + struct drm_connector connector;
>> + struct drm_bridge bridge;
>> + struct edid *edid;
>> + struct mipi_dsi_device dsi;
>> + struct i2c_client *page[MAX_DEVS];
>> + struct i2c_client *ddc_i2c;
>> + struct regulator_bulk_data supplies[2];
>> + struct drm_panel *panel;
>> + struct gpio_desc *gpio_reset;
>> + struct gpio_desc *gpio_power_down;
>> + struct gpio_desc *gpio_mode_sel;
>> + bool enabled;
>> +
>> + /* firmware file info */
>> + struct ps8640_info info;
>> + bool in_fw_update;
>> + /* for firmware update protect */
>> + struct mutex fw_mutex;
>> +};
>> +
>> +static const u8 enc_ctrl_code[6] = { 0xaa, 0x55, 0x50, 0x41, 0x52, 0x44 };
>> +static const u8 hw_chip_id[4] = { 0x00, 0x0a, 0x00, 0x30 };
>> +
>> +static inline struct ps8640 *bridge_to_ps8640(struct drm_bridge *e)
>> +{
>> + return container_of(e, struct ps8640, bridge);
>> +}
>> +
>> +static inline struct ps8640 *connector_to_ps8640(struct drm_connector *e)
>> +{
>> + return container_of(e, struct ps8640, connector);
>> +}
>> +
>> +static int ps8640_read(struct i2c_client *client, u8 reg, u8 *data,
>> + u16 data_len)
>> +{
>> + int ret;
>> + struct i2c_msg msgs[] = {
>> + {
>> + .addr = client->addr,
>> + .flags = 0,
>> + .len = 1,
>> + .buf = ®,
>> + },
>> + {
>> + .addr = client->addr,
>> + .flags = I2C_M_RD,
>> + .len = data_len,
>> + .buf = data,
>> + }
>> + };
>> +
>> + ret = i2c_transfer(client->adapter, msgs, 2);
>> +
>> + if (ret == 2)
>> + return 0;
>> + if (ret < 0)
>> + return ret;
>> + else
>> + return -EIO;
>> +}
>> +
>> +static int ps8640_write_bytes(struct i2c_client *client, const u8 *data,
>> + u16 data_len)
>> +{
>> + int ret;
>> + struct i2c_msg msg;
>> +
>> + msg.addr = client->addr;
>> + msg.flags = 0;
>> + msg.len = data_len;
>> + msg.buf = (u8 *)data;
>> +
>> + ret = i2c_transfer(client->adapter, &msg, 1);
>> + if (ret == 1)
>> + return 0;
>> + if (ret < 0)
>> + return ret;
>> + else
>> + return -EIO;
>> +}
>> +
>> +static int ps8640_write_byte(struct i2c_client *client, u8 reg, u8 data)
>> +{
>> + u8 buf[] = { reg, data };
>> +
>> + return ps8640_write_bytes(client, buf, sizeof(buf));
>> +}
>> +
>> +static void ps8640_get_mcu_fw_version(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[5];
>> + u8 fw_ver[2];
>> +
>> + ps8640_read(client, 0x4, fw_ver, sizeof(fw_ver));
>> + ps_bridge->info.version = (fw_ver[0] << 8) | fw_ver[1];
>> +
>> + DRM_INFO_ONCE("ps8640 rom fw version %d.%d\n", fw_ver[0], fw_ver[1]);
>> +}
>> +
>> +static int ps8640_bridge_unmute(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[3];
>> + u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_EN };
>> +
>> + return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
>> +}
>> +
>> +static int ps8640_bridge_mute(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[3];
>> + u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_DIS };
>> +
>> + return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
>> +}
>> +
>> +static void ps8640_pre_enable(struct drm_bridge *bridge)
>> +{
>> + struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> + struct i2c_client *client = ps_bridge->page[2];
>> + struct i2c_client *page1 = ps_bridge->page[1];
It's a bit hard to follow what page[3] or page[5] means without going to the
bottom and reading the dummy devices comment. It would be nice to have some
macros here.
>> + int err;
>> + u8 set_vdo_done, mcs_en, vstart;
>> + ktime_t timeout;
>> +
>> + if (ps_bridge->in_fw_update)
>> + return;
>> +
>> + if (ps_bridge->enabled)
>> + return;
>> +
>> + err = drm_panel_prepare(ps_bridge->panel);
>> + if (err < 0) {
>> + DRM_ERROR("failed to prepare panel: %d\n", err);
>> + return;
>> + }
>> +
>> + err = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
>> + ps_bridge->supplies);
>> + if (err < 0) {
>> + DRM_ERROR("cannot enable regulators %d\n", err);
>> + goto err_panel_unprepare;
>> + }
>> +
>> + gpiod_set_value(ps_bridge->gpio_power_down, 1);
>> + gpiod_set_value(ps_bridge->gpio_reset, 0);
>> + usleep_range(2000, 2500);
>> + gpiod_set_value(ps_bridge->gpio_reset, 1);
>> +
>> + /*
>> + * Wait for the ps8640 embed mcu ready
>> + * First wait 200ms and then check the mcu ready flag every 20ms
>> + */
>> + msleep(200);
>> +
>> + timeout = ktime_add_ms(ktime_get(), 200);
>> + for (;;) {
>> + err = ps8640_read(client, PAGE2_GPIO_H, &set_vdo_done, 1);
>> + if (err < 0) {
>> + DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", err);
>> + goto err_regulators_disable;
>> + }
>> + if ((set_vdo_done & PS_GPIO9) == PS_GPIO9)
>> + break;
>> + if (ktime_compare(ktime_get(), timeout) > 0)
>> + break;
>> + msleep(20);
>> + }
>> +
>> + msleep(50);
>> +
>> + ps8640_read(page1, PAGE1_VSTART, &vstart, 1);
>> + DRM_INFO("PS8640 PAGE1.0x6B = 0x%x\n", vstart);
>> +
>> + /**
>> + * The Manufacturer Command Set (MCS) is a device dependent interface
>> + * intended for factory programming of the display module default
>> + * parameters. Once the display module is configured, the MCS shall be
>> + * disabled by the manufacturer. Once disabled, all MCS commands are
>> + * ignored by the display interface.
>> + */
>> + ps8640_read(client, PAGE2_MCS_EN, &mcs_en, 1);
>> + ps8640_write_byte(client, PAGE2_MCS_EN, mcs_en & ~MCS_EN);
>> +
>> + if (ps_bridge->info.version == 0)
>> + ps8640_get_mcu_fw_version(ps_bridge);
>> +
>> + err = ps8640_bridge_unmute(ps_bridge);
>> + if (err)
>> + DRM_ERROR("failed to enable unmutevideo: %d\n", err);
>> + /* Switch access edp panel's edid through i2c */
>> + ps8640_write_byte(client, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
>> + ps_bridge->enabled = true;
>> +
>> + return;
>> +
>> +err_regulators_disable:
>> + regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
>> + ps_bridge->supplies);
>> +err_panel_unprepare:
>> + drm_panel_unprepare(ps_bridge->panel);
>> +}
>> +
>> +static void ps8640_enable(struct drm_bridge *bridge)
>> +{
>> + struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> + int err;
>> +
>> + err = drm_panel_enable(ps_bridge->panel);
>> + if (err < 0)
>> + DRM_ERROR("failed to enable panel: %d\n", err);
>> +}
>> +
>> +static void ps8640_disable(struct drm_bridge *bridge)
>> +{
>> + struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> + int err;
>> +
>> + err = drm_panel_disable(ps_bridge->panel);
>> + if (err < 0)
>> + DRM_ERROR("failed to disable panel: %d\n", err);
>> +}
>> +
>> +static void ps8640_post_disable(struct drm_bridge *bridge)
>> +{
>> + struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> + int err;
>> +
>> + if (ps_bridge->in_fw_update)
>> + return;
>> +
>> + if (!ps_bridge->enabled)
>> + return;
>> +
>> + ps_bridge->enabled = false;
>> +
>> + err = ps8640_bridge_mute(ps_bridge);
>> + if (err < 0)
>> + DRM_ERROR("failed to unmutevideo: %d\n", err);
>> +
>> + gpiod_set_value(ps_bridge->gpio_reset, 0);
>> + gpiod_set_value(ps_bridge->gpio_power_down, 0);
>> + err = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
>> + ps_bridge->supplies);
>> + if (err < 0)
>> + DRM_ERROR("cannot disable regulators %d\n", err);
>> +
>> + err = drm_panel_unprepare(ps_bridge->panel);
>> + if (err)
>> + DRM_ERROR("failed to unprepare panel: %d\n", err);
>> +}
>> +
>> +static int ps8640_get_modes(struct drm_connector *connector)
>> +{
>> + struct ps8640 *ps_bridge = connector_to_ps8640(connector);
>> + struct edid *edid;
>> + int num_modes = 0;
>> + bool power_off;
>> +
>> + if (ps_bridge->edid)
>> + return drm_add_edid_modes(connector, ps_bridge->edid);
>> +
>> + power_off = !ps_bridge->enabled;
>> + ps8640_pre_enable(&ps_bridge->bridge);
>> +
>> + edid = drm_get_edid(connector, ps_bridge->ddc_i2c->adapter);
See comments related to this in ps8640_probe.
>> + if (!edid)
>> + goto out;
>> +
>> + ps_bridge->edid = edid;
>> + drm_mode_connector_update_edid_property(connector, ps_bridge->edid);
>> + num_modes = drm_add_edid_modes(connector, ps_bridge->edid);
>> +
>> +out:
>> + if (power_off)
>> + ps8640_post_disable(&ps_bridge->bridge);
>> +
>> + return num_modes;
>> +}
>> +
>> +static struct drm_encoder *ps8640_best_encoder(struct drm_connector *connector)
>> +{
>> + struct ps8640 *ps_bridge = connector_to_ps8640(connector);
>> +
>> + return ps_bridge->bridge.encoder;
>> +}
We can drop the above func.
>> +
>> +static const struct drm_connector_helper_funcs ps8640_connector_helper_funcs = {
>> + .get_modes = ps8640_get_modes,
>> + .best_encoder = ps8640_best_encoder,
>> +};
>> +
>> +static enum drm_connector_status ps8640_detect(struct drm_connector *connector,
>> + bool force)
>> +{
>> + return connector_status_connected;
>> +}
>> +
>> +static const struct drm_connector_funcs ps8640_connector_funcs = {
>> + .dpms = drm_atomic_helper_connector_dpms,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .detect = ps8640_detect,
>> + .reset = drm_atomic_helper_connector_reset,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +int ps8640_bridge_attach(struct drm_bridge *bridge)
>> +{
>> + struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> + struct device *dev = &ps_bridge->page[0]->dev;
>> + struct device_node *port, *in_ep;
>> + struct device_node *dsi_node = NULL;
>> + struct mipi_dsi_host *host = NULL;
>> + int ret;
>> +
>> + ret = drm_connector_init(bridge->dev, &ps_bridge->connector,
>> + &ps8640_connector_funcs,
>> + DRM_MODE_CONNECTOR_eDP);
>> +
>> + if (ret) {
>> + DRM_ERROR("Failed to initialize connector with drm: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + drm_connector_helper_add(&ps_bridge->connector,
>> + &ps8640_connector_helper_funcs);
>> +
>> + ps_bridge->connector.dpms = DRM_MODE_DPMS_ON;
>> + drm_mode_connector_attach_encoder(&ps_bridge->connector,
>> + bridge->encoder);
>> +
>> + if (ps_bridge->panel)
>> + drm_panel_attach(ps_bridge->panel, &ps_bridge->connector);
>> +
>> + /* port at 0 is ps8640 dsi input port */
>> + port = of_graph_get_port_by_id(dev->of_node, 0);
>> + if (port) {
>> + in_ep = of_get_child_by_name(port, "endpoint");
>> + of_node_put(port);
The above 2 funcs can be done by a single func: of_graph_get_endpoint_by_regs().
>> + if (in_ep) {
>> + dsi_node = of_graph_get_remote_port_parent(in_ep);
>> + of_node_put(in_ep);
>> + }
>> + }
>> + if (dsi_node) {
>> + host = of_find_mipi_dsi_host_by_node(dsi_node);
>> + of_node_put(dsi_node);
>> + if (!host) {
>> + ret = -ENODEV;
>> + goto err;
>> + }
>> + }
>> +
We haven't created a DSI device for this yet. Don't we need to call
mipi_dsi_device_register_full() here?
>> + ps_bridge->dsi.host = host;
The code above proceeds even if we don't find a dsi host. In that
case, the host would be a NULL pointer. We shouldn't call
mipi_dsi_attach() with a NULL host. We should have returned earlier with
an error.
>> + ps_bridge->dsi.mode_flags = MIPI_DSI_MODE_VIDEO |
>> + MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
>> + ps_bridge->dsi.format = MIPI_DSI_FMT_RGB888;
>> + ps_bridge->dsi.lanes = 4;
>> + ret = mipi_dsi_attach(&ps_bridge->dsi);
>> + if (ret)
>> + goto err;
>> +
>> + return 0;
>> +err:
>> + if (ps_bridge->panel)
>> + drm_panel_detach(ps_bridge->panel);
>> + drm_connector_cleanup(&ps_bridge->connector);
>> + return ret;
>> +}
>> +
>> +static const struct drm_bridge_funcs ps8640_bridge_funcs = {
>> + .attach = ps8640_bridge_attach,
>> + .disable = ps8640_disable,
>> + .post_disable = ps8640_post_disable,
>> + .pre_enable = ps8640_pre_enable,
>> + .enable = ps8640_enable,
>> +};
>> +
>> +/* Firmware Version is returned as Major.Minor */
>> +static ssize_t ps8640_fw_version_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct ps8640 *ps_bridge = dev_get_drvdata(dev);
>> + struct ps8640_info *info = &ps_bridge->info;
>> +
>> + return scnprintf(buf, PAGE_SIZE, "%u.%u\n", info->version >> 8,
>> + info->version & 0xff);
>> +}
>> +
>> +/* Hardware Version is returned as FamilyID.VariantID */
>> +static ssize_t ps8640_hw_version_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct ps8640 *ps_bridge = dev_get_drvdata(dev);
>> + struct ps8640_info *info = &ps_bridge->info;
>> +
>> + return scnprintf(buf, PAGE_SIZE, "ps%u.%u\n", info->family_id,
>> + info->variant_id);
>> +}
>> +
>> +static int ps8640_spi_send_cmd(struct ps8640 *ps_bridge, u8 *cmd, u8 cmd_len)
>> +{
>> + struct i2c_client *client = ps_bridge->page[2];
>> + u8 i, buf[3] = { PAGE2_SWSPI_LEN, cmd_len - 1, TRIGGER_NO_READBACK };
>> + int ret;
>> +
>> + ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> + if (ret)
>> + goto err;
>> +
>> + /* write command in write port */
>> + for (i = 0; i < cmd_len; i++) {
>> + ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA, cmd[i]);
>> + if (ret)
>> + goto err_irom_disable;
>> + }
>> +
>> + ret = ps8640_write_bytes(client, buf, sizeof(buf));
>> + if (ret)
>> + goto err_irom_disable;
>> +
>> + ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> + if (ret)
>> + goto err;
>> +
>> + return 0;
>> +err_irom_disable:
>> + ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +err:
>> + dev_err(&client->dev, "send command err: %d\n", ret);
>> + return ret;
>> +}
>> +
>> +static int ps8640_wait_spi_ready(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[2];
>> + u8 spi_rdy_st;
>> + ktime_t timeout;
>> +
>> + timeout = ktime_add_ms(ktime_get(), 200);
>> + for (;;) {
>> + ps8640_read(client, PAGE2_SPI_STATUS, &spi_rdy_st, 1);
>> + if ((spi_rdy_st & SPI_READY) != SPI_READY)
>> + break;
>> +
>> + if (ktime_compare(ktime_get(), timeout) > 0) {
>> + dev_err(&client->dev, "wait spi ready timeout\n");
>> + return -EBUSY;
>> + }
>> +
>> + msleep(20);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int ps8640_wait_spi_nobusy(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[2];
>> + u8 spi_status, buf[3] = { PAGE2_SWSPI_LEN, 0, TRIGGER_READBACK };
>> + int ret;
>> + ktime_t timeout;
>> +
>> + timeout = ktime_add_ms(ktime_get(), 500);
>> + for (;;) {
>> + /* 0x05 RDSR; Read-Status-Register */
>> + ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA,
>> + READ_STATUS_REG_CMD);
>> + if (ret)
>> + goto err_send_cmd_exit;
>> +
>> + ret = ps8640_write_bytes(client, buf, 3);
>> + if (ret)
>> + goto err_send_cmd_exit;
>> +
>> + /* delay for cmd send */
>> + usleep_range(300, 500);
>> + /* wait for SPI ROM until not busy */
>> + ret = ps8640_read(client, PAGE2_SWSPI_RDATA, &spi_status, 1);
>> + if (ret)
>> + goto err_send_cmd_exit;
>> +
>> + if (!(spi_status & BUSY))
>> + break;
>> +
>> + if (ktime_compare(ktime_get(), timeout) > 0) {
>> + dev_err(&client->dev, "wait spi no busy timeout: %d\n",
>> + ret);
>> + return -EBUSY;
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +err_send_cmd_exit:
>> + dev_err(&client->dev, "send command err: %d\n", ret);
>> + return ret;
>> +}
>> +
>> +static int ps8640_wait_rom_idle(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[0];
>> + int ret;
>> +
>> + ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> + if (ret)
>> + goto exit;
>> +
>> + ret = ps8640_wait_spi_ready(ps_bridge);
>> + if (ret)
>> + goto err_spi;
>> +
>> + ret = ps8640_wait_spi_nobusy(ps_bridge);
>> + if (ret)
>> + goto err_spi;
>> +
>> + ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> + if (ret)
>> + goto exit;
>> +
>> + return 0;
>> +
>> +err_spi:
>> + ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +exit:
>> + dev_err(&client->dev, "wait ps8640 rom idle fail: %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static int ps8640_spi_dl_mode(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[2];
>> + int ret;
>> +
>> + /* switch ps8640 mode to spi dl mode */
>> + if (ps_bridge->gpio_mode_sel)
>> + gpiod_set_value(ps_bridge->gpio_mode_sel, 0);
>> +
>> + /* reset spi interface */
>> + ret = ps8640_write_byte(client, PAGE2_SW_RESET,
>> + SPI_SW_RESET | MPU_SW_RESET);
>> + if (ret)
>> + goto exit;
>> +
>> + ret = ps8640_write_byte(client, PAGE2_SW_RESET, MPU_SW_RESET);
>> + if (ret)
>> + goto exit;
>> +
>> + return 0;
>> +
>> +exit:
>> + dev_err(&client->dev, "fail reset spi interface: %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static int ps8640_rom_prepare(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[2];
>> + struct device *dev = &client->dev;
>> + u8 i, cmd[2];
>> + int ret;
>> +
>> + cmd[0] = WRITE_ENABLE_CMD;
>> + ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> + if (ret) {
>> + dev_err(dev, "failed enable-write-status-register: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + cmd[0] = WRITE_STATUS_REG_CMD;
>> + cmd[1] = CLEAR_ALL_PROTECT;
>> + ret = ps8640_spi_send_cmd(ps_bridge, cmd, 2);
>> + if (ret) {
>> + dev_err(dev, "fail disable all protection: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /* wait for SPI module ready */
>> + ret = ps8640_wait_rom_idle(ps_bridge);
>> + if (ret) {
>> + dev_err(dev, "fail wait rom idle: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> + for (i = 0; i < ARRAY_SIZE(enc_ctrl_code); i++)
>> + ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, enc_ctrl_code[i]);
>> + ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +
>> + /* Enable-Write-Status-Register */
>> + cmd[0] = WRITE_ENABLE_CMD;
>> + ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> + if (ret) {
>> + dev_err(dev, "fail enable-write-status-register: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /* chip erase command */
>> + cmd[0] = CHIP_ERASE_CMD;
>> + ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> + if (ret) {
>> + dev_err(dev, "fail disable all protection: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = ps8640_wait_rom_idle(ps_bridge);
>> + if (ret) {
>> + dev_err(dev, "fail wait rom idle: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int ps8640_check_chip_id(struct ps8640 *ps_bridge)
>> +{
>> + struct i2c_client *client = ps_bridge->page[4];
>> + u8 buf[4];
>> +
>> + ps8640_read(client, PAGE4_REV_L, buf, 4);
>> + return memcmp(buf, hw_chip_id, sizeof(buf));
>> +}
>> +
>> +static int ps8640_validate_firmware(struct ps8640 *ps_bridge,
>> + const struct firmware *fw)
>> +{
>> + struct i2c_client *client = ps_bridge->page[0];
>> + u16 fw_chip_id;
>> +
>> + /*
>> + * Get the chip_id from the firmware. Make sure that it is the
>> + * right controller to do the firmware and config update.
>> + */
>> + fw_chip_id = get_unaligned_le16(fw->data + FW_CHIP_ID_OFFSET);
>> +
>> + if (fw_chip_id != 0x8640 && ps8640_check_chip_id(ps_bridge) == 0) {
>> + dev_err(&client->dev,
>> + "chip id mismatch: fw 0x%x vs. chip 0x8640\n",
>> + fw_chip_id);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int ps8640_write_rom(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> + struct i2c_client *client = ps_bridge->page[0];
>> + struct device *dev = &client->dev;
>> + struct i2c_client *client2 = ps_bridge->page[2];
>> + struct i2c_client *client7 = ps_bridge->page[7];
>> + size_t pos, cpy_len;
>> + u8 buf[257];
>> + int ret;
>> +
>> + ps8640_write_byte(client2, PAGE2_SPI_CFG3, I2C_TO_SPI_RESET);
>> + msleep(100);
>> + ps8640_write_byte(client2, PAGE2_SPI_CFG3, 0x00);
>> +
>> + for (pos = 0; pos < fw->size; pos += cpy_len) {
>> + buf[0] = PAGE2_ROMADD_BYTE1;
>> + buf[1] = pos >> 8;
>> + buf[2] = pos >> 16;
>> + ret = ps8640_write_bytes(client2, buf, 3);
>> + if (ret)
>> + goto error;
>> + cpy_len = fw->size >= 256 + pos ? 256 : fw->size - pos;
>> + buf[0] = 0;
>> + memcpy(buf + 1, fw->data + pos, cpy_len);
>> + ret = ps8640_write_bytes(client7, buf, cpy_len + 1);
>> + if (ret)
>> + goto error;
>> +
>> + dev_dbg(dev, "fw update completed %zu / %zu bytes\n", pos,
>> + fw->size);
>> + }
>> + return 0;
>> +
>> +error:
>> + dev_err(dev, "failed write external flash, %d\n", ret);
>> + return ret;
>> +}
>> +
>> +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge)
>> +{
>> + u8 cmd[2];
>> + struct i2c_client *client = ps_bridge->page[2];
>> +
>> + /* Enable-Write-Status-Register */
>> + cmd[0] = WRITE_ENABLE_CMD;
>> + ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +
>> + /* protect BPL/BP0/BP1 */
>> + cmd[0] = WRITE_STATUS_REG_CMD;
>> + cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT;
>> + ps8640_spi_send_cmd(ps_bridge, cmd, 2);
>> +
>> + /* wait for SPI rom ready */
>> + ps8640_wait_rom_idle(ps_bridge);
>> +
>> + /* disable PS8640 mapping function */
>> + ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00);
>> +
>> + if (ps_bridge->gpio_mode_sel)
>> + gpiod_set_value(ps_bridge->gpio_mode_sel, 1);
>> + return 0;
>> +}
>> +
>> +static int ps8640_enter_bl(struct ps8640 *ps_bridge)
>> +{
>> + ps_bridge->in_fw_update = true;
>> + return ps8640_spi_dl_mode(ps_bridge);
>> +}
>> +
>> +static void ps8640_exit_bl(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> + ps8640_spi_normal_mode(ps_bridge);
>> + ps_bridge->in_fw_update = false;
>> +}
>> +
>> +static int ps8640_load_fw(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> + struct i2c_client *client = ps_bridge->page[0];
>> + struct device *dev = &client->dev;
>> + int ret;
>> + bool ps8640_status_backup = ps_bridge->enabled;
>> +
>> + ret = ps8640_validate_firmware(ps_bridge, fw);
>> + if (ret)
>> + return ret;
>> +
>> + mutex_lock(&ps_bridge->fw_mutex);
>> + if (!ps_bridge->in_fw_update) {
>> + if (!ps8640_status_backup)
>> + ps8640_pre_enable(&ps_bridge->bridge);
>> +
>> + ret = ps8640_enter_bl(ps_bridge);
>> + if (ret)
>> + goto exit;
>> + }
>> +
>> + ret = ps8640_rom_prepare(ps_bridge);
>> + if (ret)
>> + goto exit;
>> +
>> + ret = ps8640_write_rom(ps_bridge, fw);
>> +
>> +exit:
>> + if (ret)
>> + dev_err(dev, "Failed to load firmware, %d\n", ret);
>> +
>> + ps8640_exit_bl(ps_bridge, fw);
>> + if (!ps8640_status_backup)
>> + ps8640_post_disable(&ps_bridge->bridge);
>> + mutex_unlock(&ps_bridge->fw_mutex);
>> + return ret;
>> +}
>> +
>> +static ssize_t ps8640_update_fw_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct i2c_client *client = to_i2c_client(dev);
>> + struct ps8640 *ps_bridge = i2c_get_clientdata(client);
>> + const struct firmware *fw;
>> + int error;
>> +
>> + error = request_firmware(&fw, PS_FW_NAME, dev);
>> + if (error) {
>> + dev_err(dev, "Unable to open firmware %s: %d\n",
>> + PS_FW_NAME, error);
>> + return error;
>> + }
>> +
>> + error = ps8640_load_fw(ps_bridge, fw);
>> + if (error)
>> + dev_err(dev, "The firmware update failed(%d)\n", error);
>> + else
>> + dev_info(dev, "The firmware update succeeded\n");
>> +
>> + release_firmware(fw);
>> + return error ? error : count;
>> +}
>> +
>> +static DEVICE_ATTR(fw_version, S_IRUGO, ps8640_fw_version_show, NULL);
>> +static DEVICE_ATTR(hw_version, S_IRUGO, ps8640_hw_version_show, NULL);
>> +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, ps8640_update_fw_store);
>> +
>> +static struct attribute *ps8640_attrs[] = {
>> + &dev_attr_fw_version.attr,
>> + &dev_attr_hw_version.attr,
>> + &dev_attr_update_fw.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute_group ps8640_attr_group = {
>> + .attrs = ps8640_attrs,
>> +};
>> +
>> +static void ps8640_remove_sysfs_group(void *data)
>> +{
>> + struct ps8640 *ps_bridge = data;
>> +
>> + sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
>> +}
>> +
>> +static int ps8640_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct device *dev = &client->dev;
>> + struct ps8640 *ps_bridge;
>> + struct device_node *np = dev->of_node;
>> + struct device_node *port, *out_ep;
>> + struct device_node *panel_node = NULL;
>> + int ret;
>> + u32 i;
>> +
>> + ps_bridge = devm_kzalloc(dev, sizeof(*ps_bridge), GFP_KERNEL);
>> + if (!ps_bridge)
>> + return -ENOMEM;
>> +
>> + /* port at 1 is ps8640 output port */
>> + port = of_graph_get_port_by_id(np, 1);
>> + if (port) {
>> + out_ep = of_get_child_by_name(port, "endpoint");
>> + of_node_put(port);
>> + if (out_ep) {
>> + panel_node = of_graph_get_remote_port_parent(out_ep);
>> + of_node_put(out_ep);
>> + }
>> + }
>> + if (panel_node) {
>> + ps_bridge->panel = of_drm_find_panel(panel_node);
>> + of_node_put(panel_node);
>> + if (!ps_bridge->panel)
>> + return -EPROBE_DEFER;
>> + }
>> +
>> + mutex_init(&ps_bridge->fw_mutex);
>> + ps_bridge->supplies[0].supply = "vdd33";
>> + ps_bridge->supplies[1].supply = "vdd12";
>> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
>> + ps_bridge->supplies);
>> + if (ret) {
>> + dev_info(dev, "failed to get regulators: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ps_bridge->gpio_mode_sel = devm_gpiod_get_optional(&client->dev,
>> + "mode-sel",
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(ps_bridge->gpio_mode_sel)) {
>> + ret = PTR_ERR(ps_bridge->gpio_mode_sel);
>> + dev_err(dev, "cannot get mode-sel %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ps_bridge->gpio_power_down = devm_gpiod_get(&client->dev, "sleep",
>> + GPIOD_OUT_LOW);
>> + if (IS_ERR(ps_bridge->gpio_power_down)) {
>> + ret = PTR_ERR(ps_bridge->gpio_power_down);
>> + dev_err(dev, "cannot get sleep: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /*
>> + * Request the reset pin low to avoid the bridge being
>> + * initialized prematurely
>> + */
>> + ps_bridge->gpio_reset = devm_gpiod_get(&client->dev, "reset",
>> + GPIOD_OUT_LOW);
>> + if (IS_ERR(ps_bridge->gpio_reset)) {
>> + ret = PTR_ERR(ps_bridge->gpio_reset);
>> + dev_err(dev, "cannot get reset: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
>> + ps_bridge->bridge.of_node = dev->of_node;
>> +
>> + ps_bridge->page[0] = client;
>> + ps_bridge->ddc_i2c = i2c_new_dummy(client->adapter, EDID_I2C_ADDR);
I don't see why we need to create this dummy client. The drm edid helper
drm_get_edid() just needs the i2c adapter to which the client is connected.
It will internally initiate a read form the address EDID_I2C_ADDR.
I guess "drm_get_edid(connector, ps_bridge->page[0]->adapter)" should work.
>> + if (!ps_bridge->ddc_i2c) {
>> + dev_err(dev, "failed ddc_i2c dummy device, address%02x\n",
>> + EDID_I2C_ADDR);
>> + return -EBUSY;
>> + }
>> + /*
>> + * ps8640 uses multiple addresses, use dummy devices for them
>> + * page[0]: for DP control
>> + * page[1]: for VIDEO Bridge
>> + * page[2]: for control top
>> + * page[3]: for DSI Link Control1
>> + * page[4]: for MIPI Phy
>> + * page[5]: for VPLL
>> + * page[6]: for DSI Link Control2
Does this chip support 2 DSI inputs, and we're just exposing one for now?
If so, we should probably revisit the DT bindings, so that port at 2 doesn't
need to represent the 2nd DSI link.
Thanks,
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v2] staging: vc04_services: rework ioctl code path
From: Michael Zoran @ 2016-11-11 6:15 UTC (permalink / raw)
To: linux-arm-kernel
VCHIQ/vc04_services has a userland device interface
that includes ioctls. The current ioctl implementation
is a single monolithic function over 1,000+ lines
that handles 17 different ioctls through a complex
maze of switch and if statements.
The change reimplements that code path by breaking
up the code into smaller, easier to maintain functions
and uses a dispatch table to invoke the correct function.
Testing:
1. vchiq_test -f 10 and vchiq_test -p 1 were run from a native
64-bit OS(debian sid).
2. vchiq_test -f 10 and vchiq_test -p 1 where run from a 32-bit
chroot install from the same OS.
Both test cases pass.
This is V2 of this patch. Changes include:
1. More code has been moved to the dispatch routine.
The dispatch routine is now responsible for copying the top-level
data into and out of kernel space by using the data encoded in
the ioctl command number.
2. The number of parameters have been reduced for the handling
functions by giving a different prototype to ioctls that pass
no arguments.
3. Macros in linux/compat.h are now used for compatibility data
structures.
Signed-off-by: Michael Zoran <mzoran@crowfest.net>
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 1733 +++++++++++++-------
.../interface/vchiq_arm/vchiq_ioctl.h | 96 ++
2 files changed, 1200 insertions(+), 629 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 8fcd940..4eb5d73 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -503,709 +503,1181 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
&context, total_size);
}
-/****************************************************************************
-*
-* vchiq_ioctl
-*
-***************************************************************************/
+struct vchiq_ioctl_entry;
+
+struct vchiq_ioctl_call_context {
+ VCHIQ_INSTANCE_T instance;
+ unsigned int cmd;
+ unsigned long arg;
+ const struct vchiq_ioctl_entry *ioctl_entry;
+ VCHIQ_SERVICE_T *service;
+ void *prev_kmalloc;
+ bool stackmem_used;
+ unsigned long stackmem[32 / sizeof(unsigned long)];
+};
+
+static void
+vchiq_init_ioctl_call_context(struct vchiq_ioctl_call_context *ctxt)
+{
+ ctxt->service = NULL;
+ ctxt->prev_kmalloc = NULL;
+ ctxt->stackmem_used = false;
+}
+
+static void vchiq_destroy_ioctl_call_context(struct vchiq_ioctl_call_context *ctxt)
+{
+ void *prev_kmalloc = ctxt->prev_kmalloc;
+
+ while (prev_kmalloc) {
+ void *tmp = prev_kmalloc;
+
+ prev_kmalloc = *(void **)prev_kmalloc;
+ kfree(tmp);
+ }
+
+ if (ctxt->service)
+ unlock_service(ctxt->service);
+}
+
+static void *
+vchiq_ioctl_kmalloc(struct vchiq_ioctl_call_context *ctxt, size_t size)
+{
+ void *mem;
+
+ if (!ctxt->stackmem_used && size < sizeof(ctxt->stackmem)) {
+ ctxt->stackmem_used = true;
+ return ctxt->stackmem;
+ }
+
+ mem = kmalloc(size + sizeof(void *), GFP_KERNEL);
+ if (!mem)
+ return NULL;
+
+ *(void **)mem = ctxt->prev_kmalloc;
+ ctxt->prev_kmalloc = mem;
+
+ return mem + sizeof(void *);
+}
+
+typedef long (*vchiq_ioctl_no_arg_func_t)(struct vchiq_ioctl_call_context *);
+
+typedef long (*vchiq_ioctl_ptr_arg_func_t)(struct vchiq_ioctl_call_context*,
+ void *arg);
+
+struct vchiq_ioctl_entry {
+ void *func;
+ unsigned int ioctl;
+ void *aux_data;
+};
+
+static long
+vchiq_ioctl_dispatch(const struct vchiq_ioctl_entry *ioctl_entry,
+ struct file *file, unsigned int cmd, unsigned long arg)
+{
+ long ret = 0;
+ unsigned int ioctl_dir;
+ unsigned int ioctl_size;
+ struct vchiq_ioctl_call_context ctxt;
+
+ if (ioctl_entry->ioctl != cmd)
+ return -ENOTTY;
+
+ ioctl_dir = _IOC_DIR(cmd);
+ ioctl_size = _IOC_SIZE(cmd);
+
+ vchiq_init_ioctl_call_context(&ctxt);
+
+ ctxt.instance = file->private_data;
+ ctxt.cmd = cmd;
+ ctxt.arg = arg;
+ ctxt.ioctl_entry = ioctl_entry;
+
+ if (ioctl_dir == _IOC_NONE) {
+ vchiq_ioctl_no_arg_func_t func;
+
+ func = (vchiq_ioctl_no_arg_func_t)ioctl_entry->func;
+ ret = func(&ctxt);
+ } else {
+ vchiq_ioctl_ptr_arg_func_t func;
+ void *kptr;
+
+ kptr = vchiq_ioctl_kmalloc(&ctxt, ioctl_size);
+
+ if (!kptr) {
+ vchiq_destroy_ioctl_call_context(&ctxt);
+ return -ENOMEM;
+ }
+
+ if (ioctl_dir & _IOC_WRITE) {
+ if (copy_from_user(kptr, (void __user *)arg,
+ ioctl_size)) {
+ vchiq_destroy_ioctl_call_context(&ctxt);
+ return -EFAULT;
+ }
+ }
+
+ func = (vchiq_ioctl_ptr_arg_func_t)ioctl_entry->func;
+ ret = func(&ctxt, kptr);
+
+ if (ret < 0) {
+ vchiq_destroy_ioctl_call_context(&ctxt);
+ return ret;
+ }
+
+ if (ioctl_dir & _IOC_READ) {
+ if (copy_to_user((void __user *)arg, kptr,
+ ioctl_size)) {
+ vchiq_destroy_ioctl_call_context(&ctxt);
+ return -EFAULT;
+ }
+ }
+ }
+
+ vchiq_destroy_ioctl_call_context(&ctxt);
+ return ret;
+}
+
+static long vchiq_map_status(VCHIQ_STATUS_T status)
+{
+ if (status == VCHIQ_ERROR)
+ return -EIO;
+
+ if (status == VCHIQ_RETRY)
+ return -EINTR;
+
+ return 0;
+}
+
+static long
+vchiq_ioctl_shutdown(VCHIQ_INSTANCE_T instance) {
+ VCHIQ_SERVICE_T *service = NULL;
+ int i;
+
+ if (!instance->connected)
+ return 0;
+
+ /* Remove all services */
+ i = 0;
+ while ((service = next_service_by_instance(instance->state,
+ instance, &i)) != NULL) {
+ VCHIQ_STATUS_T status;
+
+ status = vchiq_remove_service(service->handle);
+ unlock_service(service);
+
+ if (status != VCHIQ_SUCCESS)
+ return vchiq_map_status(status);
+ }
+
+ /* Wake the completion thread and ask it to exit */
+ instance->closing = 1;
+ up(&instance->insert_event);
+
+ return 0;
+}
+
+static long
+vchiq_ioctl_connect(VCHIQ_INSTANCE_T instance)
+{
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ int rc;
+
+ if (instance->connected)
+ return -EINVAL;
+
+ rc = mutex_lock_interruptible(&instance->state->mutex);
+ if (rc) {
+ vchiq_log_error(vchiq_arm_log_level,
+ "vchiq: connect: could not lock mutex for state %d: %d",
+ instance->state->id, rc);
+ return -EINTR;
+ }
+
+ status = vchiq_connect_internal(instance->state, instance);
+ mutex_unlock(&instance->state->mutex);
+
+ if (status != VCHIQ_SUCCESS) {
+ vchiq_log_error(vchiq_arm_log_level,
+ "vchiq: could not connect: %d", status);
+ return vchiq_map_status(status);
+ }
+
+ instance->connected = 1;
+ return 0;
+}
+
+static long
+vchiq_ioctl_instance_adapter(struct vchiq_ioctl_call_context *ctxt)
+{
+ long (*func)(VCHIQ_INSTANCE_T) = ctxt->ioctl_entry->aux_data;
+
+ return func(ctxt->instance);
+}
+
+static long
+vchiq_ioctl_create_service(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_CREATE_SERVICE_T *args)
+{
+ VCHIQ_INSTANCE_T instance = ctxt->instance;
+ VCHIQ_SERVICE_T *service = NULL;
+ USER_SERVICE_T *user_service = NULL;
+ void *userdata;
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ int srvstate;
+
+ user_service = kmalloc(sizeof(USER_SERVICE_T), GFP_KERNEL);
+ if (!user_service)
+ return -ENOMEM;
+
+ if (args->is_open) {
+ if (!instance->connected) {
+ kfree(user_service);
+ return -ENOTCONN;
+ }
+ srvstate = VCHIQ_SRVSTATE_OPENING;
+ } else {
+ srvstate =
+ instance->connected ?
+ VCHIQ_SRVSTATE_LISTENING :
+ VCHIQ_SRVSTATE_HIDDEN;
+ }
+
+ userdata = args->params.userdata;
+ args->params.callback = service_callback;
+ args->params.userdata = user_service;
+ service = vchiq_add_service_internal(
+ instance->state,
+ &args->params, srvstate,
+ instance, user_service_free);
+
+ if (!service) {
+ kfree(user_service);
+ return -EEXIST;
+ }
+
+ user_service->service = service;
+ user_service->userdata = userdata;
+ user_service->instance = instance;
+ user_service->is_vchi = (args->is_vchi != 0);
+ user_service->dequeue_pending = 0;
+ user_service->close_pending = 0;
+ user_service->message_available_pos = instance->completion_remove - 1;
+ user_service->msg_insert = 0;
+ user_service->msg_remove = 0;
+ sema_init(&user_service->insert_event, 0);
+ sema_init(&user_service->remove_event, 0);
+ sema_init(&user_service->close_event, 0);
+
+ if (args->is_open) {
+ status = vchiq_open_service_internal
+ (service, instance->pid);
+ if (status != VCHIQ_SUCCESS) {
+ vchiq_remove_service(service->handle);
+ return vchiq_map_status(status);
+ }
+ }
+
+ args->handle = service->handle;
+ return 0;
+}
+
+static long
+vchiq_ioctl_close_service(VCHIQ_SERVICE_T *service)
+{
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ USER_SERVICE_T *user_service;
+
+ user_service = (USER_SERVICE_T *)service->base.userdata;
+
+ /*
+ * close_pending is false on first entry, and when the
+ * wait in vchiq_close_service has been interrupted.
+ */
+ if (!user_service->close_pending) {
+ status = vchiq_close_service(service->handle);
+ if (status != VCHIQ_SUCCESS)
+ return vchiq_map_status(status);
+ }
+
+ /*
+ * close_pending is true once the underlying service
+ * has been closed until the client library calls the
+ * CLOSE_DELIVERED ioctl, signalling close_event.
+ */
+ if (user_service->close_pending &&
+ down_interruptible(&user_service->close_event))
+ return vchiq_map_status(VCHIQ_RETRY);
+
+ return 0;
+}
+
+static long
+vchiq_ioctl_remove_service(VCHIQ_SERVICE_T *service) {
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ USER_SERVICE_T *user_service;
+
+ user_service = (USER_SERVICE_T *)service->base.userdata;
+
+ /*
+ * close_pending is false on first entry, and when the
+ * wait in vchiq_close_service has been interrupted.
+ */
+ if (!user_service->close_pending) {
+ status = vchiq_remove_service(service->handle);
+ if (status != VCHIQ_SUCCESS)
+ return vchiq_map_status(status);
+ }
+
+ /*
+ * close_pending is true once the underlying service
+ * has been closed until the client library calls the
+ * CLOSE_DELIVERED ioctl, signaling close_event.
+ */
+ if (user_service->close_pending &&
+ down_interruptible(&user_service->close_event))
+ return vchiq_map_status(VCHIQ_RETRY);
+
+ return 0;
+}
+
+static long
+vchiq_ioctl_use_service(VCHIQ_SERVICE_T *service) {
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+
+ status = vchiq_use_service_internal(service);
+
+ if (status != VCHIQ_SUCCESS) {
+ vchiq_log_error(vchiq_susp_log_level,
+ "vchiq_ioctl: cmd VCHIQ_IOC_USE_SERVICE returned error %d for service %c%c%c%c:%03d",
+ status,
+ VCHIQ_FOURCC_AS_4CHARS(
+ service->base.fourcc),
+ service->client_id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static long
+vchiq_ioctl_release_service(VCHIQ_SERVICE_T *service) {
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+
+ status = vchiq_release_service_internal(service);
+
+ if (status != VCHIQ_SUCCESS) {
+ vchiq_log_error(vchiq_susp_log_level,
+ "vchiq_ioctl: cmd VCHIQ_IOC_RELEASE_SERVICE returned error %d for service %c%c%c%c:%03d",
+ status,
+ VCHIQ_FOURCC_AS_4CHARS(
+ service->base.fourcc),
+ service->client_id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static long
+vchiq_ioctl_service_adapter(struct vchiq_ioctl_call_context *ctxt)
+{
+ long (*func)(VCHIQ_SERVICE_T *) = ctxt->ioctl_entry->aux_data;
+ VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)ctxt->arg;
+ long ret;
+
+ ctxt->service = find_service_for_instance(ctxt->instance, handle);
+
+ if (!ctxt->service)
+ return -EINVAL;
+
+ ret = func(ctxt->service);
+
+ return ret;
+}
+
+static long
+vchiq_ioctl_queue_message(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_QUEUE_MESSAGE_T *args) {
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ VCHIQ_ELEMENT_T *elements;
+
+ ctxt->service = find_service_for_instance(ctxt->instance, args->handle);
+
+ if (!ctxt->service)
+ return -EINVAL;
+
+ elements = vchiq_ioctl_kmalloc(ctxt,
+ args->count * sizeof(VCHIQ_ELEMENT_T));
+
+ if (!elements)
+ return -ENOMEM;
+
+ if (copy_from_user(elements, args->elements,
+ args->count * sizeof(VCHIQ_ELEMENT_T)))
+ return -EINVAL;
+
+ status = vchiq_ioc_queue_message(args->handle,
+ elements, args->count);
+
+ return vchiq_map_status(status);
+}
+
+static long
+vchiq_ioctl_bulk_transfer(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_QUEUE_BULK_TRANSFER_T *args,
+ VCHIQ_BULK_DIR_T dir)
+{
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ struct bulk_waiter_node *waiter = NULL;
+ void *userdata = args->userdata;
+
+ ctxt->service = find_service_for_instance(ctxt->instance, args->handle);
+ if (!ctxt->service)
+ return -EINVAL;
+
+ if (args->mode == VCHIQ_BULK_MODE_BLOCKING) {
+ waiter = kzalloc(sizeof(struct bulk_waiter_node), GFP_KERNEL);
+ if (!waiter)
+ return -ENOMEM;
+
+ userdata = &waiter->bulk_waiter;
+ } else if (args->mode == VCHIQ_BULK_MODE_WAITING) {
+ struct list_head *pos;
+
+ mutex_lock(&ctxt->instance->bulk_waiter_list_mutex);
+
+ list_for_each(pos, &ctxt->instance->bulk_waiter_list)
+ {
+ if (list_entry(pos, struct bulk_waiter_node,
+ list)->pid == current->pid) {
+ waiter = list_entry(pos,
+ struct bulk_waiter_node,
+ list);
+ list_del(pos);
+ break;
+ }
+ }
+ mutex_unlock(&ctxt->instance->bulk_waiter_list_mutex);
+ if (!waiter) {
+ vchiq_log_error(vchiq_arm_log_level,
+ "no bulk_waiter found for pid %d",
+ current->pid);
+ return -ESRCH;
+ }
+ vchiq_log_info(vchiq_arm_log_level,
+ "found bulk_waiter %pK for pid %d", waiter,
+ current->pid);
+ userdata = &waiter->bulk_waiter;
+ }
+
+ status = vchiq_bulk_transfer(args->handle,
+ VCHI_MEM_HANDLE_INVALID,
+ args->data,
+ args->size,
+ userdata,
+ args->mode,
+ dir);
+
+ if (!waiter)
+ return vchiq_map_status(status);
+
+ if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
+ !waiter->bulk_waiter.bulk) {
+ if (waiter->bulk_waiter.bulk) {
+ /*
+ * Cancel the signal when the transfer
+ * completes.
+ */
+ spin_lock(&bulk_waiter_spinlock);
+ waiter->bulk_waiter.bulk->userdata = NULL;
+ spin_unlock(&bulk_waiter_spinlock);
+ }
+ kfree(waiter);
+ } else {
+ waiter->pid = current->pid;
+ mutex_lock(&ctxt->instance->bulk_waiter_list_mutex);
+ list_add(&waiter->list, &ctxt->instance->bulk_waiter_list);
+ mutex_unlock(&ctxt->instance->bulk_waiter_list_mutex);
+ vchiq_log_info(vchiq_arm_log_level,
+ "saved bulk_waiter %pK for pid %d",
+ waiter, current->pid);
+
+ args->mode = VCHIQ_BULK_MODE_WAITING;
+ }
+
+ return vchiq_map_status(status);
+}
+
+static long
+vchiq_ioctl_bulk_transmit(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_QUEUE_BULK_TRANSFER_T *args) {
+ return vchiq_ioctl_bulk_transfer(ctxt, args, VCHIQ_BULK_TRANSMIT);
+};
+
+static long
+vchiq_ioctl_bulk_receive(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_QUEUE_BULK_TRANSFER_T *args) {
+ return vchiq_ioctl_bulk_transfer(ctxt, args, VCHIQ_BULK_RECEIVE);
+};
+
+typedef bool (*vchiq_get_msgbuf_ptr_callback_t)(void *context,
+ unsigned int msgbufcount,
+ void __user **msgbuf);
+
+typedef bool (*vchiq_put_completion_callback_t)(void *context,
+ unsigned int num,
+ VCHIQ_COMPLETION_DATA_T *completion);
+
+static long
+vchiq_ioctl_await_completion_internal(VCHIQ_INSTANCE_T instance,
+ unsigned int count,
+ unsigned int msgbufsize,
+ unsigned int *argmsgbufcount,
+ void *context,
+ vchiq_get_msgbuf_ptr_callback_t get_msgbuf,
+ vchiq_put_completion_callback_t put_completion)
+{
+ long ret = 0;
+ int msgbufcount;
+
+ DEBUG_INITIALISE(g_state.local);
+
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+ if (!instance->connected)
+ return -ENOTCONN;
+
+ mutex_lock(&instance->completion_mutex);
+
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ while ((instance->completion_remove == instance->completion_insert)
+ && !instance->closing) {
+ int rc;
+
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ mutex_unlock(&instance->completion_mutex);
+ rc = down_interruptible(&instance->insert_event);
+ mutex_lock(&instance->completion_mutex);
+ if (rc != 0) {
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ vchiq_log_info(vchiq_arm_log_level,
+ "AWAIT_COMPLETION interrupted");
+ mutex_unlock(&instance->completion_mutex);
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ return -EINTR;
+ }
+ }
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+ /*
+ * A read memory barrier is needed to stop prefetch of a stale
+ * completion record.
+ */
+ rmb();
+
+ msgbufcount = *argmsgbufcount;
+ for (ret = 0; ret < count; ret++) {
+ VCHIQ_COMPLETION_DATA_T *completion;
+ VCHIQ_SERVICE_T *service;
+ USER_SERVICE_T *user_service;
+ VCHIQ_HEADER_T *header;
+
+ if (instance->completion_remove == instance->completion_insert)
+ break;
+ completion = &instance->completions[
+ instance->completion_remove &
+ (MAX_COMPLETIONS - 1)];
+
+ service = completion->service_userdata;
+ user_service = service->base.userdata;
+ completion->service_userdata = user_service->userdata;
+
+ header = completion->header;
+ if (header) {
+ void __user *msgbuf;
+ int msglen;
+
+ msglen = header->size + sizeof(VCHIQ_HEADER_T);
+ /* This must be a VCHIQ-style service */
+ if (msgbufsize < msglen) {
+ vchiq_log_error(
+ vchiq_arm_log_level,
+ "header %pK: msgbufsize %x < msglen %x",
+ header, msgbufsize,
+ msglen);
+ WARN(1, "invalid message size\n");
+ if (ret == 0)
+ ret = -EMSGSIZE;
+ break;
+ }
+ if (msgbufcount <= 0)
+ /*
+ * Stall here for lack of a
+ * buffer for the message.
+ */
+ break;
+ /* Get the pointer from user space */
+ msgbufcount--;
+
+ if (get_msgbuf(context, msgbufcount, &msgbuf)) {
+ if (ret == 0)
+ ret = -EFAULT;
+ break;
+ }
+
+ /* Copy the message to user space */
+ if (copy_to_user(msgbuf, header,
+ msglen) != 0) {
+ if (ret == 0)
+ ret = -EFAULT;
+ break;
+ }
+
+ /*
+ * Now it has been copied, the message
+ * can be released.
+ */
+ vchiq_release_message(service->handle,
+ header);
+
+ /*
+ * The completion must point to the
+ * msgbuf.
+ */
+ completion->header = msgbuf;
+ }
+
+ if ((completion->reason ==
+ VCHIQ_SERVICE_CLOSED) &&
+ !instance->use_close_delivered)
+ unlock_service(service);
+
+ if (put_completion(context, (unsigned int)ret, completion)) {
+ if (ret == 0)
+ ret = -EFAULT;
+ }
+
+ instance->completion_remove++;
+ }
+
+ *argmsgbufcount = msgbufcount;
+
+ if (ret != 0)
+ up(&instance->remove_event);
+ mutex_unlock(&instance->completion_mutex);
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+ return ret;
+}
+
+static bool
+vchiq_ioctl_get_msgbuf(void *context,
+ unsigned int msgbufcount,
+ void __user **msgbuf)
+{
+ VCHIQ_AWAIT_COMPLETION_T *args = (VCHIQ_AWAIT_COMPLETION_T *)context;
+
+ return !!copy_from_user(msgbuf,
+ (const void __user *)
+ &args->msgbufs[msgbufcount],
+ sizeof(*msgbuf));
+}
+
+static bool
+vchiq_ioctl_put_completion(void *context,
+ unsigned int num,
+ VCHIQ_COMPLETION_DATA_T *completion)
+{
+ VCHIQ_AWAIT_COMPLETION_T *args = (VCHIQ_AWAIT_COMPLETION_T *)context;
+
+ return !!copy_to_user((void __user *)(
+ (size_t)args->buf +
+ num * sizeof(VCHIQ_COMPLETION_DATA_T)),
+ completion,
+ sizeof(VCHIQ_COMPLETION_DATA_T));
+}
+
+static long
+vchiq_ioctl_await_completion(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_AWAIT_COMPLETION_T *args)
+{
+ return vchiq_ioctl_await_completion_internal(ctxt->instance,
+ args->count,
+ args->msgbufsize,
+ &args->msgbufcount,
+ args,
+ vchiq_ioctl_get_msgbuf,
+ vchiq_ioctl_put_completion);
+}
+
+static long
+vchiq_ioctl_dequeue_message(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_DEQUEUE_MESSAGE_T *args)
+{
+ USER_SERVICE_T *user_service;
+ VCHIQ_HEADER_T *header;
+ long ret;
+
+ DEBUG_INITIALISE(g_state.local);
+
+ ctxt->service = find_service_for_instance(ctxt->instance,
+ args->handle);
+ if (!ctxt->service)
+ return -EINVAL;
+
+ user_service = (USER_SERVICE_T *)ctxt->service->base.userdata;
+ if (user_service->is_vchi == 0)
+ return -EINVAL;
+
+ spin_lock(&msg_queue_spinlock);
+ if (user_service->msg_remove == user_service->msg_insert) {
+ if (!args->blocking) {
+ spin_unlock(&msg_queue_spinlock);
+ DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+ return -EWOULDBLOCK;
+ }
+ user_service->dequeue_pending = 1;
+ do {
+ spin_unlock(&msg_queue_spinlock);
+ DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+ if (down_interruptible(
+ &user_service->insert_event) != 0) {
+ vchiq_log_info(vchiq_arm_log_level,
+ "DEQUEUE_MESSAGE interrupted");
+ return -EINTR;
+ }
+ spin_lock(&msg_queue_spinlock);
+ } while (user_service->msg_remove == user_service->msg_insert);
+ }
+
+ BUG_ON((int)(user_service->msg_insert - user_service->msg_remove) < 0);
+
+ header = user_service->msg_queue[user_service->msg_remove &
+ (MSG_QUEUE_SIZE - 1)];
+ user_service->msg_remove++;
+ spin_unlock(&msg_queue_spinlock);
+
+ up(&user_service->remove_event);
+ if (!header)
+ return -ENOTCONN;
+
+ if (header->size > args->bufsize) {
+ vchiq_log_error(vchiq_arm_log_level,
+ "header %pK: bufsize %x < size %x",
+ header, args->bufsize, header->size);
+ WARN(1, "invalid size\n");
+ return -EMSGSIZE;
+ }
+
+ if (!args->buf)
+ return -EMSGSIZE;
+
+ if (copy_to_user(args->buf,
+ header->data,
+ header->size))
+ return -EFAULT;
+
+ ret = header->size;
+ vchiq_release_message(ctxt->service->handle, header);
+
+ DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+ return ret;
+}
+
static long
-vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+vchiq_ioctl_get_client_handle(struct vchiq_ioctl_call_context *ctxt)
{
- VCHIQ_INSTANCE_T instance = file->private_data;
+ VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)ctxt->arg;
+
+ return vchiq_get_client_id(handle);
+}
+
+static long
+vchiq_ioctl_get_config(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_GET_CONFIG_T *args)
+{
+ VCHIQ_CONFIG_T config;
VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
- VCHIQ_SERVICE_T *service = NULL;
- long ret = 0;
- int i, rc;
- DEBUG_INITIALISE(g_state.local)
- vchiq_log_trace(vchiq_arm_log_level,
- "vchiq_ioctl - instance %pK, cmd %s, arg %lx",
- instance,
- ((_IOC_TYPE(cmd) == VCHIQ_IOC_MAGIC) &&
- (_IOC_NR(cmd) <= VCHIQ_IOC_MAX)) ?
- ioctl_names[_IOC_NR(cmd)] : "<invalid>", arg);
-
- switch (cmd) {
- case VCHIQ_IOC_SHUTDOWN:
- if (!instance->connected)
- break;
+ if (args->config_size > sizeof(config))
+ return -EFAULT;
- /* Remove all services */
- i = 0;
- while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
- status = vchiq_remove_service(service->handle);
- unlock_service(service);
- if (status != VCHIQ_SUCCESS)
- break;
- }
- service = NULL;
+ status = vchiq_get_config(ctxt->instance, args->config_size, &config);
- if (status == VCHIQ_SUCCESS) {
- /* Wake the completion thread and ask it to exit */
- instance->closing = 1;
- up(&instance->insert_event);
- }
+ if (status != VCHIQ_SUCCESS)
+ return vchiq_map_status(status);
- break;
+ if (copy_to_user((void __user *)args->pconfig,
+ &config, args->config_size))
+ return -EFAULT;
- case VCHIQ_IOC_CONNECT:
- if (instance->connected) {
- ret = -EINVAL;
- break;
- }
- rc = mutex_lock_interruptible(&instance->state->mutex);
- if (rc != 0) {
- vchiq_log_error(vchiq_arm_log_level,
- "vchiq: connect: could not lock mutex for "
- "state %d: %d",
- instance->state->id, rc);
- ret = -EINTR;
- break;
- }
- status = vchiq_connect_internal(instance->state, instance);
- mutex_unlock(&instance->state->mutex);
+ return 0;
+}
- if (status == VCHIQ_SUCCESS)
- instance->connected = 1;
- else
- vchiq_log_error(vchiq_arm_log_level,
- "vchiq: could not connect: %d", status);
- break;
+static long
+vchiq_ioctl_set_service_option(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_SET_SERVICE_OPTION_T *args)
+{
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
- case VCHIQ_IOC_CREATE_SERVICE: {
- VCHIQ_CREATE_SERVICE_T args;
- USER_SERVICE_T *user_service = NULL;
- void *userdata;
- int srvstate;
+ ctxt->service = find_service_for_instance(ctxt->instance, args->handle);
+ if (!ctxt->service)
+ return -EINVAL;
- if (copy_from_user
- (&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
+ status = vchiq_set_service_option(
+ args->handle, args->option, args->value);
- user_service = kmalloc(sizeof(USER_SERVICE_T), GFP_KERNEL);
- if (!user_service) {
- ret = -ENOMEM;
- break;
- }
+ return vchiq_map_status(status);
+}
- if (args.is_open) {
- if (!instance->connected) {
- ret = -ENOTCONN;
- kfree(user_service);
- break;
- }
- srvstate = VCHIQ_SRVSTATE_OPENING;
- } else {
- srvstate =
- instance->connected ?
- VCHIQ_SRVSTATE_LISTENING :
- VCHIQ_SRVSTATE_HIDDEN;
- }
+static long
+vchiq_ioctl_dump_phys_mem(struct vchiq_ioctl_call_context *ctxt,
+ VCHIQ_DUMP_MEM_T *args)
+{
+ dump_phys_mem(args->virt_addr, args->num_bytes);
- userdata = args.params.userdata;
- args.params.callback = service_callback;
- args.params.userdata = user_service;
- service = vchiq_add_service_internal(
- instance->state,
- &args.params, srvstate,
- instance, user_service_free);
-
- if (service != NULL) {
- user_service->service = service;
- user_service->userdata = userdata;
- user_service->instance = instance;
- user_service->is_vchi = (args.is_vchi != 0);
- user_service->dequeue_pending = 0;
- user_service->close_pending = 0;
- user_service->message_available_pos =
- instance->completion_remove - 1;
- user_service->msg_insert = 0;
- user_service->msg_remove = 0;
- sema_init(&user_service->insert_event, 0);
- sema_init(&user_service->remove_event, 0);
- sema_init(&user_service->close_event, 0);
-
- if (args.is_open) {
- status = vchiq_open_service_internal
- (service, instance->pid);
- if (status != VCHIQ_SUCCESS) {
- vchiq_remove_service(service->handle);
- service = NULL;
- ret = (status == VCHIQ_RETRY) ?
- -EINTR : -EIO;
- break;
- }
- }
+ return 0;
+}
- if (copy_to_user((void __user *)
- &(((VCHIQ_CREATE_SERVICE_T __user *)
- arg)->handle),
- (const void *)&service->handle,
- sizeof(service->handle)) != 0) {
- ret = -EFAULT;
- vchiq_remove_service(service->handle);
- }
+static long
+vchiq_ioctl_lib_version(struct vchiq_ioctl_call_context *ctxt)
+{
+ unsigned int lib_version = (unsigned int)ctxt->arg;
- service = NULL;
- } else {
- ret = -EEXIST;
- kfree(user_service);
- }
- } break;
+ if (lib_version < VCHIQ_VERSION_MIN)
+ return -EINVAL;
- case VCHIQ_IOC_CLOSE_SERVICE: {
- VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
-
- service = find_service_for_instance(instance, handle);
- if (service != NULL) {
- USER_SERVICE_T *user_service =
- (USER_SERVICE_T *)service->base.userdata;
- /* close_pending is false on first entry, and when the
- wait in vchiq_close_service has been interrupted. */
- if (!user_service->close_pending) {
- status = vchiq_close_service(service->handle);
- if (status != VCHIQ_SUCCESS)
- break;
- }
+ if (lib_version >= VCHIQ_VERSION_CLOSE_DELIVERED)
+ ctxt->instance->use_close_delivered = 1;
- /* close_pending is true once the underlying service
- has been closed until the client library calls the
- CLOSE_DELIVERED ioctl, signalling close_event. */
- if (user_service->close_pending &&
- down_interruptible(&user_service->close_event))
- status = VCHIQ_RETRY;
- }
- else
- ret = -EINVAL;
- } break;
+ return 0;
+}
- case VCHIQ_IOC_REMOVE_SERVICE: {
- VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
-
- service = find_service_for_instance(instance, handle);
- if (service != NULL) {
- USER_SERVICE_T *user_service =
- (USER_SERVICE_T *)service->base.userdata;
- /* close_pending is false on first entry, and when the
- wait in vchiq_close_service has been interrupted. */
- if (!user_service->close_pending) {
- status = vchiq_remove_service(service->handle);
- if (status != VCHIQ_SUCCESS)
- break;
- }
+static long
+vchiq_ioctl_close_delivered(struct vchiq_ioctl_call_context *ctxt)
+{
+ VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)ctxt->arg;
- /* close_pending is true once the underlying service
- has been closed until the client library calls the
- CLOSE_DELIVERED ioctl, signalling close_event. */
- if (user_service->close_pending &&
- down_interruptible(&user_service->close_event))
- status = VCHIQ_RETRY;
- }
- else
- ret = -EINVAL;
- } break;
+ ctxt->service = find_closed_service_for_instance(ctxt->instance, handle);
+ if (!ctxt->service)
+ return -EINVAL;
- case VCHIQ_IOC_USE_SERVICE:
- case VCHIQ_IOC_RELEASE_SERVICE: {
- VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
+ close_delivered((USER_SERVICE_T *)ctxt->service->base.userdata);
+ return 0;
+}
- service = find_service_for_instance(instance, handle);
- if (service != NULL) {
- status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
- vchiq_use_service_internal(service) :
- vchiq_release_service_internal(service);
- if (status != VCHIQ_SUCCESS) {
- vchiq_log_error(vchiq_susp_log_level,
- "%s: cmd %s returned error %d for "
- "service %c%c%c%c:%03d",
- __func__,
- (cmd == VCHIQ_IOC_USE_SERVICE) ?
- "VCHIQ_IOC_USE_SERVICE" :
- "VCHIQ_IOC_RELEASE_SERVICE",
- status,
- VCHIQ_FOURCC_AS_4CHARS(
- service->base.fourcc),
- service->client_id);
- ret = -EINVAL;
- }
- } else
- ret = -EINVAL;
- } break;
+#define VCHIQ_MK_IOCTL(__ioctl, __func, __aux_data) \
+ [_IOC_NR(__ioctl)] = {.func = __func, .ioctl = __ioctl, .aux_data = __aux_data}
+
+static const struct vchiq_ioctl_entry vchiq_ioctl_table[] = {
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_CONNECT, vchiq_ioctl_instance_adapter, vchiq_ioctl_connect),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_SHUTDOWN, vchiq_ioctl_instance_adapter, vchiq_ioctl_shutdown),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_CREATE_SERVICE, vchiq_ioctl_create_service, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_CLOSE_SERVICE, vchiq_ioctl_service_adapter, vchiq_ioctl_close_service),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_REMOVE_SERVICE, vchiq_ioctl_service_adapter, vchiq_ioctl_remove_service),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_USE_SERVICE, vchiq_ioctl_service_adapter, vchiq_ioctl_use_service),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_RELEASE_SERVICE, vchiq_ioctl_service_adapter, vchiq_ioctl_release_service),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_QUEUE_MESSAGE, vchiq_ioctl_queue_message, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_QUEUE_BULK_TRANSMIT, vchiq_ioctl_bulk_transmit, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_QUEUE_BULK_RECEIVE, vchiq_ioctl_bulk_receive, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_AWAIT_COMPLETION, vchiq_ioctl_await_completion, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_DEQUEUE_MESSAGE, vchiq_ioctl_dequeue_message, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_GET_CLIENT_ID, vchiq_ioctl_get_client_handle, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_GET_CONFIG, vchiq_ioctl_get_config, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_SET_SERVICE_OPTION, vchiq_ioctl_set_service_option, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_DUMP_PHYS_MEM, vchiq_ioctl_dump_phys_mem, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_LIB_VERSION, vchiq_ioctl_lib_version, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_CLOSE_DELIVERED, vchiq_ioctl_close_delivered, NULL),
+};
- case VCHIQ_IOC_QUEUE_MESSAGE: {
- VCHIQ_QUEUE_MESSAGE_T args;
- if (copy_from_user
- (&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
+/**************************************************************************
+ *
+ * vchiq_ioctl
+ *
+ **************************************************************************/
+static long
+vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ VCHIQ_INSTANCE_T instance = file->private_data;
+ long ret = 0;
+ unsigned int ioctl_nr;
- service = find_service_for_instance(instance, args.handle);
+ ioctl_nr = _IOC_NR(cmd);
- if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
- /* Copy elements into kernel space */
- VCHIQ_ELEMENT_T elements[MAX_ELEMENTS];
- if (copy_from_user(elements, args.elements,
- args.count * sizeof(VCHIQ_ELEMENT_T)) == 0)
- status = vchiq_ioc_queue_message
- (args.handle,
- elements, args.count);
- else
- ret = -EFAULT;
- } else {
- ret = -EINVAL;
- }
- } break;
+ vchiq_log_trace(vchiq_arm_log_level,
+ "vchiq_ioctl - instance %pK, cmd %s, arg %lx",
+ instance,
+ ((_IOC_TYPE(cmd) == VCHIQ_IOC_MAGIC) &&
+ (ioctl_nr <= VCHIQ_IOC_MAX)) ?
+ ioctl_names[ioctl_nr] : "<invalid>", arg);
- case VCHIQ_IOC_QUEUE_BULK_TRANSMIT:
- case VCHIQ_IOC_QUEUE_BULK_RECEIVE: {
- VCHIQ_QUEUE_BULK_TRANSFER_T args;
- struct bulk_waiter_node *waiter = NULL;
- VCHIQ_BULK_DIR_T dir =
- (cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT) ?
- VCHIQ_BULK_TRANSMIT : VCHIQ_BULK_RECEIVE;
-
- if (copy_from_user
- (&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
+ if (ioctl_nr > VCHIQ_IOC_MAX) {
+ ret = -ENOTTY;
+ } else {
+ ret = vchiq_ioctl_dispatch(&vchiq_ioctl_table[ioctl_nr],
+ file, cmd, arg);
+ }
- service = find_service_for_instance(instance, args.handle);
- if (!service) {
- ret = -EINVAL;
- break;
- }
+ if ((ret < 0) && (ret != -EINTR) && (ret != -EWOULDBLOCK))
+ vchiq_log_info(vchiq_arm_log_level,
+ " ioctl instance %lx, cmd %s, %ld",
+ (unsigned long)instance,
+ (ioctl_nr <= VCHIQ_IOC_MAX) ?
+ ioctl_names[ioctl_nr] :
+ "<invalid>",
+ ret);
+ else
+ vchiq_log_trace(vchiq_arm_log_level,
+ " ioctl instance %lx, cmd %s, %ld",
+ (unsigned long)instance,
+ (ioctl_nr <= VCHIQ_IOC_MAX) ?
+ ioctl_names[ioctl_nr] :
+ "<invalid>",
+ ret);
- if (args.mode == VCHIQ_BULK_MODE_BLOCKING) {
- waiter = kzalloc(sizeof(struct bulk_waiter_node),
- GFP_KERNEL);
- if (!waiter) {
- ret = -ENOMEM;
- break;
- }
- args.userdata = &waiter->bulk_waiter;
- } else if (args.mode == VCHIQ_BULK_MODE_WAITING) {
- struct list_head *pos;
- mutex_lock(&instance->bulk_waiter_list_mutex);
- list_for_each(pos, &instance->bulk_waiter_list) {
- if (list_entry(pos, struct bulk_waiter_node,
- list)->pid == current->pid) {
- waiter = list_entry(pos,
- struct bulk_waiter_node,
- list);
- list_del(pos);
- break;
- }
- }
- mutex_unlock(&instance->bulk_waiter_list_mutex);
- if (!waiter) {
- vchiq_log_error(vchiq_arm_log_level,
- "no bulk_waiter found for pid %d",
- current->pid);
- ret = -ESRCH;
- break;
- }
- vchiq_log_info(vchiq_arm_log_level,
- "found bulk_waiter %pK for pid %d", waiter,
- current->pid);
- args.userdata = &waiter->bulk_waiter;
- }
- status = vchiq_bulk_transfer
- (args.handle,
- VCHI_MEM_HANDLE_INVALID,
- args.data, args.size,
- args.userdata, args.mode,
- dir);
- if (!waiter)
- break;
- if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
- !waiter->bulk_waiter.bulk) {
- if (waiter->bulk_waiter.bulk) {
- /* Cancel the signal when the transfer
- ** completes. */
- spin_lock(&bulk_waiter_spinlock);
- waiter->bulk_waiter.bulk->userdata = NULL;
- spin_unlock(&bulk_waiter_spinlock);
- }
- kfree(waiter);
- } else {
- const VCHIQ_BULK_MODE_T mode_waiting =
- VCHIQ_BULK_MODE_WAITING;
- waiter->pid = current->pid;
- mutex_lock(&instance->bulk_waiter_list_mutex);
- list_add(&waiter->list, &instance->bulk_waiter_list);
- mutex_unlock(&instance->bulk_waiter_list_mutex);
- vchiq_log_info(vchiq_arm_log_level,
- "saved bulk_waiter %pK for pid %d",
- waiter, current->pid);
-
- if (copy_to_user((void __user *)
- &(((VCHIQ_QUEUE_BULK_TRANSFER_T __user *)
- arg)->mode),
- (const void *)&mode_waiting,
- sizeof(mode_waiting)) != 0)
- ret = -EFAULT;
- }
- } break;
+ return ret;
+}
- case VCHIQ_IOC_AWAIT_COMPLETION: {
- VCHIQ_AWAIT_COMPLETION_T args;
+#if defined(CONFIG_COMPAT)
- DEBUG_TRACE(AWAIT_COMPLETION_LINE);
- if (!instance->connected) {
- ret = -ENOTCONN;
- break;
- }
+static long
+vchiq_ioctl_compat_create_service(struct vchiq_ioctl_call_context *ctxt,
+ struct vchiq_create_service32 *args32)
+{
+ VCHIQ_CREATE_SERVICE_T args;
+ long ret;
- if (copy_from_user(&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
+ args.params.fourcc = args32->params.fourcc;
+ args.params.callback = NULL;
+ args.params.userdata = compat_ptr(args32->params.userdata);
+ args.params.version = args32->params.version;
+ args.params.version_min = args32->params.version_min;
+ args.is_open = args32->is_open;
+ args.is_vchi = args32->is_vchi;
+ args.handle = args32->handle;
- mutex_lock(&instance->completion_mutex);
+ ret = vchiq_ioctl_create_service(ctxt, &args);
- DEBUG_TRACE(AWAIT_COMPLETION_LINE);
- while ((instance->completion_remove ==
- instance->completion_insert)
- && !instance->closing) {
- int rc;
- DEBUG_TRACE(AWAIT_COMPLETION_LINE);
- mutex_unlock(&instance->completion_mutex);
- rc = down_interruptible(&instance->insert_event);
- mutex_lock(&instance->completion_mutex);
- if (rc != 0) {
- DEBUG_TRACE(AWAIT_COMPLETION_LINE);
- vchiq_log_info(vchiq_arm_log_level,
- "AWAIT_COMPLETION interrupted");
- ret = -EINTR;
- break;
- }
- }
- DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ if (ret >= 0)
+ args32->handle = args.handle;
- /* A read memory barrier is needed to stop prefetch of a stale
- ** completion record
- */
- rmb();
-
- if (ret == 0) {
- int msgbufcount = args.msgbufcount;
- for (ret = 0; ret < args.count; ret++) {
- VCHIQ_COMPLETION_DATA_T *completion;
- VCHIQ_SERVICE_T *service;
- USER_SERVICE_T *user_service;
- VCHIQ_HEADER_T *header;
- if (instance->completion_remove ==
- instance->completion_insert)
- break;
- completion = &instance->completions[
- instance->completion_remove &
- (MAX_COMPLETIONS - 1)];
-
- service = completion->service_userdata;
- user_service = service->base.userdata;
- completion->service_userdata =
- user_service->userdata;
-
- header = completion->header;
- if (header) {
- void __user *msgbuf;
- int msglen;
-
- msglen = header->size +
- sizeof(VCHIQ_HEADER_T);
- /* This must be a VCHIQ-style service */
- if (args.msgbufsize < msglen) {
- vchiq_log_error(
- vchiq_arm_log_level,
- "header %pK: msgbufsize %x < msglen %x",
- header, args.msgbufsize,
- msglen);
- WARN(1, "invalid message "
- "size\n");
- if (ret == 0)
- ret = -EMSGSIZE;
- break;
- }
- if (msgbufcount <= 0)
- /* Stall here for lack of a
- ** buffer for the message. */
- break;
- /* Get the pointer from user space */
- msgbufcount--;
- if (copy_from_user(&msgbuf,
- (const void __user *)
- &args.msgbufs[msgbufcount],
- sizeof(msgbuf)) != 0) {
- if (ret == 0)
- ret = -EFAULT;
- break;
- }
-
- /* Copy the message to user space */
- if (copy_to_user(msgbuf, header,
- msglen) != 0) {
- if (ret == 0)
- ret = -EFAULT;
- break;
- }
-
- /* Now it has been copied, the message
- ** can be released. */
- vchiq_release_message(service->handle,
- header);
+ return ret;
+}
- /* The completion must point to the
- ** msgbuf. */
- completion->header = msgbuf;
- }
+static long
+vchiq_ioctl_compat_queue_message(struct vchiq_ioctl_call_context *ctxt,
+ struct vchiq_queue_message32 *args32) {
+ VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
+ VCHIQ_ELEMENT_T *elements;
+ struct vchiq_element32 *elements32;
+ unsigned int i;
- if ((completion->reason ==
- VCHIQ_SERVICE_CLOSED) &&
- !instance->use_close_delivered)
- unlock_service(service);
-
- if (copy_to_user((void __user *)(
- (size_t)args.buf +
- ret * sizeof(VCHIQ_COMPLETION_DATA_T)),
- completion,
- sizeof(VCHIQ_COMPLETION_DATA_T)) != 0) {
- if (ret == 0)
- ret = -EFAULT;
- break;
- }
+ ctxt->service = find_service_for_instance(ctxt->instance, args32->handle);
- instance->completion_remove++;
- }
+ if (!ctxt->service)
+ return -EINVAL;
- if (msgbufcount != args.msgbufcount) {
- if (copy_to_user((void __user *)
- &((VCHIQ_AWAIT_COMPLETION_T *)arg)->
- msgbufcount,
- &msgbufcount,
- sizeof(msgbufcount)) != 0) {
- ret = -EFAULT;
- }
- }
- }
+ elements = vchiq_ioctl_kmalloc(ctxt,
+ args32->count * sizeof(VCHIQ_ELEMENT_T));
- if (ret != 0)
- up(&instance->remove_event);
- mutex_unlock(&instance->completion_mutex);
- DEBUG_TRACE(AWAIT_COMPLETION_LINE);
- } break;
+ if (!elements)
+ return -ENOMEM;
- case VCHIQ_IOC_DEQUEUE_MESSAGE: {
- VCHIQ_DEQUEUE_MESSAGE_T args;
- USER_SERVICE_T *user_service;
- VCHIQ_HEADER_T *header;
+ elements32 =
+ vchiq_ioctl_kmalloc(ctxt,
+ args32->count * sizeof(struct vchiq_element32));
- DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
- if (copy_from_user
- (&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
- service = find_service_for_instance(instance, args.handle);
- if (!service) {
- ret = -EINVAL;
- break;
- }
- user_service = (USER_SERVICE_T *)service->base.userdata;
- if (user_service->is_vchi == 0) {
- ret = -EINVAL;
- break;
- }
+ if (!elements32)
+ return -ENOMEM;
- spin_lock(&msg_queue_spinlock);
- if (user_service->msg_remove == user_service->msg_insert) {
- if (!args.blocking) {
- spin_unlock(&msg_queue_spinlock);
- DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
- ret = -EWOULDBLOCK;
- break;
- }
- user_service->dequeue_pending = 1;
- do {
- spin_unlock(&msg_queue_spinlock);
- DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
- if (down_interruptible(
- &user_service->insert_event) != 0) {
- vchiq_log_info(vchiq_arm_log_level,
- "DEQUEUE_MESSAGE interrupted");
- ret = -EINTR;
- break;
- }
- spin_lock(&msg_queue_spinlock);
- } while (user_service->msg_remove ==
- user_service->msg_insert);
+ if (copy_from_user(elements32, compat_ptr(args32->elements),
+ args32->count * sizeof(struct vchiq_element32)))
+ return -EINVAL;
- if (ret)
- break;
- }
+ for (i = 0; i < args32->count; i++) {
+ elements[i].data = compat_ptr(elements32[i].data);
+ elements[i].size = elements32[i].size;
+ }
- BUG_ON((int)(user_service->msg_insert -
- user_service->msg_remove) < 0);
+ status = vchiq_ioc_queue_message(args32->handle,
+ elements, args32->count);
- header = user_service->msg_queue[user_service->msg_remove &
- (MSG_QUEUE_SIZE - 1)];
- user_service->msg_remove++;
- spin_unlock(&msg_queue_spinlock);
+ return vchiq_map_status(status);
+}
- up(&user_service->remove_event);
- if (header == NULL)
- ret = -ENOTCONN;
- else if (header->size <= args.bufsize) {
- /* Copy to user space if msgbuf is not NULL */
- if ((args.buf == NULL) ||
- (copy_to_user((void __user *)args.buf,
- header->data,
- header->size) == 0)) {
- ret = header->size;
- vchiq_release_message(
- service->handle,
- header);
- } else
- ret = -EFAULT;
- } else {
- vchiq_log_error(vchiq_arm_log_level,
- "header %pK: bufsize %x < size %x",
- header, args.bufsize, header->size);
- WARN(1, "invalid size\n");
- ret = -EMSGSIZE;
- }
- DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
- } break;
+static long
+vchiq_ioctl_compat_bulk_transfer(struct vchiq_ioctl_call_context *ctxt,
+ struct vchiq_queue_bulk_transfer32 *args32)
+{
+ VCHIQ_QUEUE_BULK_TRANSFER_T args;
+ long ret;
- case VCHIQ_IOC_GET_CLIENT_ID: {
- VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
+ args.handle = args32->handle;
+ args.data = compat_ptr(args32->data);
+ args.size = args32->size;
+ args.userdata = compat_ptr(args32->userdata);
+ args.mode = args32->mode;
- ret = vchiq_get_client_id(handle);
- } break;
+ if (ctxt->cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT32)
+ ret = vchiq_ioctl_bulk_transmit(ctxt, &args);
+ else
+ ret = vchiq_ioctl_bulk_receive(ctxt, &args);
- case VCHIQ_IOC_GET_CONFIG: {
- VCHIQ_GET_CONFIG_T args;
- VCHIQ_CONFIG_T config;
+ args32->mode = args.mode;
- if (copy_from_user(&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
- if (args.config_size > sizeof(config)) {
- ret = -EINVAL;
- break;
- }
- status = vchiq_get_config(instance, args.config_size, &config);
- if (status == VCHIQ_SUCCESS) {
- if (copy_to_user((void __user *)args.pconfig,
- &config, args.config_size) != 0) {
- ret = -EFAULT;
- break;
- }
- }
- } break;
+ return ret;
+}
- case VCHIQ_IOC_SET_SERVICE_OPTION: {
- VCHIQ_SET_SERVICE_OPTION_T args;
+static bool
+vchiq_ioctl_compat_get_msgbuf(void *context,
+ unsigned int msgbufcount,
+ void __user **msgbuf)
+{
+ struct vchiq_await_completion32 *args32 =
+ (struct vchiq_await_completion32 *)context;
+ u32 msgbuf32;
+
+ if (copy_from_user(&msgbuf32,
+ compat_ptr(args32->msgbufs) +
+ (sizeof(u32) * msgbufcount),
+ sizeof(msgbuf32)))
+ return true;
+
+ *msgbuf = compat_ptr(msgbuf32);
+ return false;
+}
- if (copy_from_user(
- &args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
+static bool
+vchiq_ioctl_compat_put_completion(void *context,
+ unsigned int num,
+ VCHIQ_COMPLETION_DATA_T *completion)
+{
+ struct vchiq_await_completion32 *args32 =
+ (struct vchiq_await_completion32 *)context;
+ struct vchiq_completion_data32 completion32;
+
+ completion32.reason = completion->reason;
+ completion32.header = ptr_to_compat(completion->header);
+ completion32.service_userdata =
+ ptr_to_compat(completion->service_userdata);
+ completion32.bulk_userdata =
+ ptr_to_compat(completion->bulk_userdata);
+
+ return !!copy_to_user(compat_ptr(args32->buf) +
+ num * sizeof(struct vchiq_completion_data32),
+ &completion32,
+ sizeof(struct vchiq_completion_data32));
+}
- service = find_service_for_instance(instance, args.handle);
- if (!service) {
- ret = -EINVAL;
- break;
- }
+static long
+vchiq_ioctl_compat_await_completion(struct vchiq_ioctl_call_context *ctxt,
+ struct vchiq_await_completion32 *args32)
+{
+ return vchiq_ioctl_await_completion_internal(ctxt->instance,
+ args32->count,
+ args32->msgbufsize,
+ &args32->msgbufcount,
+ args32,
+ vchiq_ioctl_compat_get_msgbuf,
+ vchiq_ioctl_compat_put_completion);
+}
- status = vchiq_set_service_option(
- args.handle, args.option, args.value);
- } break;
+static long
+vchiq_ioctl_compat_dequeue_message(struct vchiq_ioctl_call_context *ctxt,
+ struct vchiq_dequeue_message32 *args32)
+{
+ VCHIQ_DEQUEUE_MESSAGE_T args;
- case VCHIQ_IOC_DUMP_PHYS_MEM: {
- VCHIQ_DUMP_MEM_T args;
+ args.handle = args32->handle;
+ args.blocking = args32->blocking;
+ args.bufsize = args32->bufsize;
+ args.buf = compat_ptr(args32->buf);
- if (copy_from_user
- (&args, (const void __user *)arg,
- sizeof(args)) != 0) {
- ret = -EFAULT;
- break;
- }
- dump_phys_mem(args.virt_addr, args.num_bytes);
- } break;
+ return vchiq_ioctl_dequeue_message(ctxt, &args);
+}
- case VCHIQ_IOC_LIB_VERSION: {
- unsigned int lib_version = (unsigned int)arg;
+static long
+vchiq_ioctl_compat_get_config(struct vchiq_ioctl_call_context *ctxt,
+ struct vchiq_get_config32 *args32)
+{
+ VCHIQ_GET_CONFIG_T args;
- if (lib_version < VCHIQ_VERSION_MIN)
- ret = -EINVAL;
- else if (lib_version >= VCHIQ_VERSION_CLOSE_DELIVERED)
- instance->use_close_delivered = 1;
- } break;
+ args.pconfig = compat_ptr(args32->pconfig);
+ args.config_size = args32->config_size;
- case VCHIQ_IOC_CLOSE_DELIVERED: {
- VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
+ return vchiq_ioctl_get_config(ctxt, &args);
+}
- service = find_closed_service_for_instance(instance, handle);
- if (service != NULL) {
- USER_SERVICE_T *user_service =
- (USER_SERVICE_T *)service->base.userdata;
- close_delivered(user_service);
- }
- else
- ret = -EINVAL;
- } break;
+static long
+vchiq_ioctl_compat_dump_phys_mem(struct vchiq_ioctl_call_context *ctxt,
+ struct vchiq_dump_mem32 *args32)
+{
+ dump_phys_mem(compat_ptr(args32->virt_addr), args32->num_bytes);
+ return 0;
+}
- default:
- ret = -ENOTTY;
- break;
- }
+static const struct vchiq_ioctl_entry vchiq_ioctl_compat_table[] = {
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_CREATE_SERVICE32, vchiq_ioctl_compat_create_service, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_QUEUE_MESSAGE32, vchiq_ioctl_compat_queue_message, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_QUEUE_BULK_TRANSMIT32, vchiq_ioctl_compat_bulk_transfer, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_QUEUE_BULK_RECEIVE32, vchiq_ioctl_compat_bulk_transfer, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_AWAIT_COMPLETION32, vchiq_ioctl_compat_await_completion, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_DEQUEUE_MESSAGE32, vchiq_ioctl_compat_dequeue_message, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_GET_CONFIG32, vchiq_ioctl_compat_get_config, NULL),
+ VCHIQ_MK_IOCTL(VCHIQ_IOC_DUMP_PHYS_MEM32, vchiq_ioctl_compat_dump_phys_mem, NULL),
+};
- if (service)
- unlock_service(service);
+static long
+vchiq_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ VCHIQ_INSTANCE_T instance = file->private_data;
+ long ret = 0;
+ unsigned int ioctl_nr;
- if (ret == 0) {
- if (status == VCHIQ_ERROR)
- ret = -EIO;
- else if (status == VCHIQ_RETRY)
- ret = -EINTR;
+ ioctl_nr = _IOC_NR(cmd);
+
+ vchiq_log_trace(vchiq_arm_log_level,
+ "vchiq_ioctl(compat) - instance %pK, cmd %s, arg %lx",
+ instance,
+ ((_IOC_TYPE(cmd) == VCHIQ_IOC_MAGIC) &&
+ (ioctl_nr <= VCHIQ_IOC_MAX)) ?
+ ioctl_names[ioctl_nr] : "<invalid>", arg);
+
+ if (ioctl_nr > VCHIQ_IOC_MAX) {
+ ret = -ENOTTY;
+ } else if (ioctl_nr < ARRAY_SIZE(vchiq_ioctl_compat_table) &&
+ vchiq_ioctl_compat_table[ioctl_nr].func) {
+ ret = vchiq_ioctl_dispatch(&vchiq_ioctl_compat_table[ioctl_nr],
+ file, cmd, arg);
+
+ } else {
+ ret = vchiq_ioctl_dispatch(&vchiq_ioctl_table[ioctl_nr],
+ file, cmd, arg);
}
- if ((status == VCHIQ_SUCCESS) && (ret < 0) && (ret != -EINTR) &&
- (ret != -EWOULDBLOCK))
+ if ((ret < 0) && (ret != -EINTR) && (ret != -EWOULDBLOCK))
vchiq_log_info(vchiq_arm_log_level,
- " ioctl instance %lx, cmd %s -> status %d, %ld",
- (unsigned long)instance,
- (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
- ioctl_names[_IOC_NR(cmd)] :
- "<invalid>",
- status, ret);
+ " ioctl(compat) instance %lx, cmd %s, %ld",
+ (unsigned long)instance,
+ (ioctl_nr <= VCHIQ_IOC_MAX) ?
+ ioctl_names[ioctl_nr] :
+ "<invalid>",
+ ret);
else
vchiq_log_trace(vchiq_arm_log_level,
- " ioctl instance %lx, cmd %s -> status %d, %ld",
- (unsigned long)instance,
- (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
- ioctl_names[_IOC_NR(cmd)] :
+ " ioctl(compat) instance %lx, cmd %s, %ld",
+ (unsigned long)instance,
+ (ioctl_nr <= VCHIQ_IOC_MAX) ?
+ ioctl_names[ioctl_nr] :
"<invalid>",
- status, ret);
+ ret);
return ret;
}
+#endif
+
/****************************************************************************
*
* vchiq_open
@@ -1660,6 +2132,9 @@ static const struct file_operations
vchiq_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = vchiq_ioctl,
+#if defined(CONFIG_COMPAT)
+ .compat_ioctl = vchiq_ioctl_compat,
+#endif
.open = vchiq_open,
.release = vchiq_release,
.read = vchiq_read
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
index 6137ae9..1681b77 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
@@ -35,6 +35,9 @@
#define VCHIQ_IOCTLS_H
#include <linux/ioctl.h>
+#if defined(CONFIG_COMPAT)
+#include <linux/compat.h>
+#endif
#include "vchiq_if.h"
#define VCHIQ_IOC_MAGIC 0xc4
@@ -128,4 +131,97 @@ typedef struct {
#define VCHIQ_IOC_CLOSE_DELIVERED _IO(VCHIQ_IOC_MAGIC, 17)
#define VCHIQ_IOC_MAX 17
+#if defined(CONFIG_COMPAT)
+
+struct vchiq_element32 {
+ compat_uptr_t data;
+ unsigned int size;
+};
+
+struct vchiq_service_base32 {
+ int fourcc;
+ compat_uptr_t callback;
+ compat_uptr_t userdata;
+};
+
+struct vchiq_service_params32 {
+ int fourcc;
+ compat_uptr_t callback;
+ compat_uptr_t userdata;
+ short version; /* Increment for non-trivial changes */
+ short version_min; /* Update for incompatible changes */
+};
+
+struct vchiq_create_service32 {
+ struct vchiq_service_params32 params;
+ int is_open;
+ int is_vchi;
+ unsigned int handle; /* OUT */
+};
+
+struct vchiq_queue_message32 {
+ unsigned int handle;
+ unsigned int count;
+ compat_uptr_t elements;
+};
+
+struct vchiq_queue_bulk_transfer32 {
+ unsigned int handle;
+ compat_uptr_t data;
+ unsigned int size;
+ compat_uptr_t userdata;
+ VCHIQ_BULK_MODE_T mode;
+};
+
+struct vchiq_completion_data32 {
+ VCHIQ_REASON_T reason;
+ compat_uptr_t header;
+ compat_uptr_t service_userdata;
+ compat_uptr_t bulk_userdata;
+};
+
+struct vchiq_await_completion32 {
+ unsigned int count;
+ compat_uptr_t buf;
+ unsigned int msgbufsize;
+ unsigned int msgbufcount; /* IN/OUT */
+ compat_uptr_t msgbufs;
+};
+
+struct vchiq_dequeue_message32 {
+ unsigned int handle;
+ int blocking;
+ unsigned int bufsize;
+ compat_uptr_t buf;
+};
+
+struct vchiq_get_config32 {
+ unsigned int config_size;
+ compat_uptr_t pconfig;
+};
+
+struct vchiq_dump_mem32 {
+ compat_uptr_t virt_addr;
+ u32 num_bytes;
+};
+
+#define VCHIQ_IOC_CREATE_SERVICE32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 2, struct vchiq_create_service32)
+#define VCHIQ_IOC_QUEUE_MESSAGE32 \
+ _IOW(VCHIQ_IOC_MAGIC, 4, struct vchiq_queue_message32)
+#define VCHIQ_IOC_QUEUE_BULK_TRANSMIT32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 5, struct vchiq_queue_bulk_transfer32)
+#define VCHIQ_IOC_QUEUE_BULK_RECEIVE32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 6, struct vchiq_queue_bulk_transfer32)
+#define VCHIQ_IOC_AWAIT_COMPLETION32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 7, struct vchiq_await_completion32)
+#define VCHIQ_IOC_DEQUEUE_MESSAGE32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 8, struct vchiq_dequeue_message32)
+#define VCHIQ_IOC_GET_CONFIG32 \
+ _IOWR(VCHIQ_IOC_MAGIC, 10, struct vchiq_get_config32)
+#define VCHIQ_IOC_DUMP_PHYS_MEM32 \
+ _IOW(VCHIQ_IOC_MAGIC, 15, struct vchiq_dump_mem32)
+
+#endif
+
#endif
--
2.10.2
^ permalink raw reply related
* [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
From: Vignesh R @ 2016-11-11 6:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <30CF3CC1-A4F1-4C31-B51F-A1AB1D352AF5@gmail.com>
On Friday 11 November 2016 09:00 AM, John Syne wrote:
>
>> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
>>
>> Hi,
>>
>> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>>> OK, then back to my original question. Given that these DT properties are supported in the driver
>>>
>>
>> Below properties are supported by only by ti_am3335x_adc driver and not
>> ti_am335x_tsc driver. As author of this patch pointed out in another
>> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
>> see a use case where these values needs to be tweaked for tsc channels,
>> therefore it does not make sense to be DT properties.
> Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing.
> Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.
There are 4 wire, 5 wire and 8 wire touchscreens. Therefore there is no
way to know in advance, how many channels are available for adc. IMO,
this is not a SoC specific configuration but board dependent. Hence, all
channel related configurations need to be in board specific dts files
and not in top level dtsi files.
>
> tscadc: tscadc at 44e0d000 {
> compatible = "ti,am3359-tscadc";
> reg = <0x44e0d000 0x1000>;
> interrupt-parent = <&intc>;
> interrupts = <16>;
> ti,hwmods = "adc_tsc";
> status = "disabled";
> dmas = <&edma 53 0>, <&edma 57 0>;
> dma-names = "fifo0", "fifo1?;
>
> tsc {
> compatible = "ti,am3359-tsc";
> };
> am335x_adc: adc {
> #io-channel-cells = <1>;
> ti,adc-channels = <0 1 2 3 4 5 6>;
> ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
> ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> compatible = "ti,am3359-adc";
> };
> };
>
> [1]https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
>
--
Regards
Vignesh
^ permalink raw reply
* [clk:clk-qcom-8994 2/2] include/linux/module.h:213:1: error: expected ',' or '; ' before 'extern'
From: Bastian Köcher @ 2016-11-11 7:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110234515.GO16026@codeaurora.org>
Thanks for doing that :)
On Donnerstag, 10. November 2016 15:45:15 CET Stephen Boyd wrote:
> On 11/11, kbuild test robot wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-qcom-8994
> > head: cc800227108710c8f02255e61659b956b041eec3
> > commit: cc800227108710c8f02255e61659b956b041eec3 [2/2] clk: qcom: Add support for msm8994 global clock controller
> > config: i386-allmodconfig (attached as .config)
> > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> > reproduce:
> > git checkout cc800227108710c8f02255e61659b956b041eec3
> > # save the attached .config to linux build tree
> > make ARCH=i386
> >
> > All error/warnings (new ones prefixed by >>):
> >
> > In file included from drivers/clk/qcom/gcc-msm8994.c:20:0:
> > >> include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
> > extern const typeof(name) __mod_##type##__##name##_device_table \
> > ^
> > >> drivers/clk/qcom/gcc-msm8994.c:2265:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
> > MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
>
> Urgh that's sad. Too bad MODULE_DEVICE_TABLE doesn't have
> something in it in the !MODULE case to cause compilation problems
> like this to come out. I'll go fix this up.
>
>
^ permalink raw reply
* [PATCH v4 1/5] arm64: perf: Basic uncore counter support for Cavium ThunderX SOC
From: Jan Glauber @ 2016-11-11 7:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110165405.GH4418@leverpostej>
On Thu, Nov 10, 2016 at 04:54:06PM +0000, Mark Rutland wrote:
> > +/*
> > + * Some notes about the various counters supported by this "uncore" PMU
> > + * and the design:
> > + *
> > + * All counters are 64 bit long.
> > + * There are no overflow interrupts.
> > + * Counters are summarized per node/socket.
> > + * Most devices appear as separate PCI devices per socket with the exception
> > + * of OCX TLK which appears as one PCI device per socket and contains several
> > + * units with counters that are merged.
>
> As a general note, as I commented on the QC L2 PMU driver [1,2], we need
> to figure out if we should be aggregating physical PMUs or not.
As said before, although it would be possible to create separate PMUs
for each unit, the individual counters are not interesting. For example
we are not interested in individual counters of Tag-and-data unit 0..7,
we just want the global view.
> Judging by subsequent patches, each unit has individual counters and
> controls, and thus we cannot atomically read/write counters or controls
> across them. As such, I do not think we should aggregate them, and
> should expose them separately to userspace.
That sounds like just moving the problem of aggregating the counters to
user-space. And would make the results even worse, if the user needs
several calls to summarize the counters, given how slow a perf counter
read is.
> That will simplify a number of things (e.g. the CPU migration code no
> longer has to iterate over a list of units).
Sure, it simplifies the kernel part, but it moves the cost to the user.
^ permalink raw reply
* [PATCH v5 00/23] Support qcom's HSIC USB and rewrite USB2 HS support
From: Peter Chen @ 2016-11-11 7:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <147682389932.20076.4186408979552866325@sboyd-linaro>
On Tue, Oct 18, 2016 at 01:51:39PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-10-18 02:31:40)
> > On Mon, Oct 17, 2016 at 06:56:13PM -0700, Stephen Boyd wrote:
> > > I've also sent separate patches for other minor pieces to make this
> > > all work. The full tree can be found here[2], hacks and all to get
> > > things working. I've tested this on the db410c, apq8074 dragonboard,
> > > and ifc6410 with configfs gadgets and otg cables.
> > >
> > > Patches based on v4.8-rc1
>
> Oops should be v4.9-rc1 here.
>
> > >
> > > Changes from v4:
> > > * Picked up Acks from Rob
> > > * Updated HS phy init sequence DT property to restrict it to offsets
> >
> > I remembered that you got all my acks for chipidea patches, right? I did
> > not check for this series.
>
> Sorry I've added in one more patch
>
> usb: chipidea: Emulate OTGSC interrupt enable path
>
> to fix extcon interrupt emulation even further.
>
> >
> > Besides, the patch "gpu: Remove depends on RESET_CONTROLLER when not a
> > provider" [1] still not be accepted, I need this patch to be merged
> > first, then apply your chipidea part, otherwise, there is a building
> > warning.
> >
> > [1] https://patchwork.kernel.org/patch/9322583/
>
> Yes, I'm going to resend that patch now. I hope that David will apply it
> for -rc2.
Stephen, just a mind. I have rebased Greg's usb-next tree (v4.9-rc3 on
it), your GPU fix is still not there.
--
Best Regards,
Peter Chen
^ permalink raw reply
* [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Sudeep Holla @ 2016-11-11 7:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMj3TYLkvfGbq6BKqr+9i6mjArjRPuqeAN1-rGO2OhoSyw@mail.gmail.com>
On 10/11/16 19:03, Olof Johansson wrote:
> On Thu, Nov 10, 2016 at 6:34 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 10/11/16 14:12, Rob Herring wrote:
>>>
>>> On Thu, Nov 10, 2016 at 4:26 AM, Sudeep Holla <sudeep.holla@arm.com>
>>> wrote:
>>>>
>>>>
>>>>
>>>> On 10/11/16 01:22, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Wed, Nov 02, 2016 at 10:52:09PM -0600, Sudeep Holla wrote:
>>>>>>
>>>>>>
>>>>>> This patch adds specific compatible to support legacy SCPI protocol.
>>>>>>
>>>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
>>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> index d1882c4540d0..ebd03fc93135 100644
>>>>>> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> @@ -7,7 +7,9 @@ by Linux to initiate various system control and power
>>>>>> operations.
>>>>>>
>>>>>> Required properties:
>>>>>>
>>>>>> -- compatible : should be "arm,scpi"
>>>>>> +- compatible : should be
>>>>>> + * "arm,scpi" : For implementations complying to SCPI v1.0 or
>>>>>> above
>>>>>> + * "arm,legacy-scpi" : For implementations complying pre SCPI
>>>>>> v1.0
>>>>>
>>>>>
>>>>>
>>>>> I'd prefer that we explicitly enumerate the old versions. Are there
>>>>> many?
>>>>>
>>>>
>>>> I understand your concern, but this legacy SCPI protocol was not
>>>> officially released. It was just WIP which vendors picked up from very
>>>> early releases. Since they are not numbered, it's hard to have specific
>>>> compatibles with different versions until v1.0. That's one of the reason
>>>> to retain platform specific compatible so that we can add any quirks
>>>> based on them if needed.
>>>>
>>>> I will probably add these information in the commit log so that it's
>>>> clear why we can't do version based compatible.
>>>
>>>
>>> This is exactly my point. By enumerate, I meant having platform
>>> specific compatibles. Having "arm,legacy-scpi" is pointless because
>>> who knows what version they followed and they may all be different.
>>>
>>
>> OK, but IIUC Olof's concern wanted a generic one along with the platform
>> specific compatible which kind of makes sense as so far we have seen
>> some commonality between Amlogic and Rockchip.
>>
>> E.g. Amlogic follows most of the legacy protocol though it deviates in
>> couple of things which we can handle with platform specific compatible
>> (in the following patch in the series). When another user(Rockchip ?)
>> make use of this legacy protocol, we can start using those platform
>> specific compatible for deviations only.
>>
>> Is that not acceptable ?
>
> If there's no shared legacy feature set, then it's probably less
> useful to have a shared less precise compatible value.
>
There is and will be some shared feature set for sure. At the least the
standard command set will be shared.
> What the main point I was trying to get across was that we shouldn't
> expand the generic binding with per-vendor compatible fields, instead
> we should have those as extensions on the side.
>
Yes I get the point. We will have per-vendor compatibles for handle the
deviations but generic one to handle the shared set.
> I'm also a little apprehensive of using "legacy", it goes in the same
> bucket as "misc". At some point 1.0 will be legacy too, etc.
>
True and I agree, how about "arm,scpi-pre-1.0" instead ?
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH] ata: xgene: Enable NCQ support for APM X-Gene SATA controller hardware v1.1
From: Rameshwar Sahu @ 2016-11-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109164531.GA14630@htj.duckdns.org>
Hi Tejun,
On Wed, Nov 9, 2016 at 10:15 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Wed, Sep 14, 2016 at 04:15:00PM +0530, Rameshwar Sahu wrote:
>> > @@ -821,8 +823,6 @@ static int xgene_ahci_probe(struct platform_device
>> > *pdev)
>> > dev_warn(&pdev->dev, "%s: Error reading
>> > device info. Assume version1\n",
>> > __func__);
>> > version = XGENE_AHCI_V1;
>> > - } else if (info->valid & ACPI_VALID_CID) {
>> > - version = XGENE_AHCI_V2;
>
> Can you please explain this part a bit? Everything else looks good to
> me.
Here we should not assume XGENE_AHCI_V2 always in case of having valid
_CID in ACPI table.
I need to remove this assumption because V1_1 has also valid _CID for
backward compatibly with v1.
>
> Thanks.
>
> --
> tejun
^ permalink raw reply
* [PATCH] staging: vc04_services: fix setup_timer.cocci warnings
From: kbuild test robot @ 2016-11-11 8:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201611111652.TBnh32bx%fengguang.wu@intel.com>
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1817:2-12: Use setup_timer function for function on line 1818.
Use setup_timer function instead of initializing timer with the function
and data fields
Generated by: scripts/coccinelle/api/setup_timer.cocci
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
Please take the patch only if it's a positive warning. Thanks!
vchiq_arm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1814,9 +1814,8 @@ vchiq_arm_init_state(VCHIQ_STATE_T *stat
arm_state->suspend_timer_timeout = SUSPEND_TIMER_TIMEOUT_MS;
arm_state->suspend_timer_running = 0;
- init_timer(&arm_state->suspend_timer);
- arm_state->suspend_timer.data = (unsigned long)(state);
- arm_state->suspend_timer.function = suspend_timer_callback;
+ setup_timer(&arm_state->suspend_timer, suspend_timer_callback,
+ (unsigned long)(state));
arm_state->first_connect = 0;
^ permalink raw reply
* PM regression with LED changes in next-20161109
From: Hans de Goede @ 2016-11-11 8:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110204852.GA31728@amd>
Hi,
On 10-11-16 21:48, Pavel Machek wrote:
> Hi!
>
>>>> It seems that we should get back to your initial approach. i.e. only
>>>> brightness changes caused by hardware should be reported.
>>>
>>> I don't think enabling poll() here is good idea. Some hardware won't
>>> be able to tell you that it changed the state. Returning maximum
>>> brightness trigger is going to use seems easier/better.
>>
>> The idea here is to allow userspace to poll() on the brightness
>> sysfs atrribute to detect changes autonomously done by the hardware,
>> such as e.g. happens on both Dell and Thinkpad laptops when pressing
>> the keyboard backlight cycle hotkey. Note that these keys do not
>> generate key-press events, the cycling through the brightness levels
>> (including off) is done entirely in firmware.
>
> Ok, so you can do that for keyboard backlight on thinkpad... I guess
> you handle that as a special trigger on the keyboard leds?
No, as said this is all done in firmware, as in this is all dealt
with by (presumably) the acpi-ec (acpi-embedded-controller) the kernel
does not do anything here, the key is "hardwired" to control the
keyboard backlight from the kernels pov.
> Can other
> triggers, such as heartbeat, be assigned to that "led"?
>
>> But we do get other ACPI events for this which we can use to let
>> userspace know this happens, which is something which user-
>> interfaces which allow control over the kbd backlight want to know.
>
> Yes, you can do that for keyboard backlight... but on thinkpads there
> are more leds, such as battery led. That can blink on battery low, and
> I don't think you can read the current status from hardware.
Well the battery LED does not show up under /sys/class/led so that
is not relevant for this situation, anyways ...
> Getting current state of led blinking with cpu trigger is also not
> quite a good idea.
I agree with you that it would be better if reading the brightness
sysfs attribute would always return the max brightness for LEDs
which are blinking or have a trigger set. But it seems that Jacek
disagrees, I will leave further discussion of this up to you and
Jacek.
> So IMO this should not be done in generic code. Instead,
> kbd-backlight trigger should have special attribute, and that one
> should be pollable.
Again there is no kbd-backlight trigger.
>> I understand that we will not always be able to do this, here is the
>> Documentation/ABI/testing/sysfs-class-led text I have in mind:
>>
>> The file supports poll() to detect changes, changes are only
>> signalled when this file is written or when the hardware /
>> firmware changes the brightness itself and the driver can detect
>> this. Changes done by kernel triggers / software blinking are
>> not signalled.
>>
>> Note the "and the driver can detect this" language, that has been there
>> since v1 of the poll() notification patch since I already expected not
>> all hardware to be able to signal this.
>
> Lets move it to separate attribute, for triggers that can do that,
> please.
As explained above this has nothing to do with triggers...
Regards,
Hans
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox