* [PATCH 02/03] pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS
From: Laurent Pinchart @ 2014-01-31 1:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140129231019.22655.41456.sendpatchset@w520>
Hi Magnus,
Thank you for the patch.
On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
>
> Create a new group for the USB0 OVC/VBUS pin by itself. This
> allows us to monitor PWEN as GPIO on the Lager board.
>
> Signed-off-by: Magnus Damm <damm@opensource.se>
> ---
>
> drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
10:23:32.000000000
> +0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
> static const unsigned int usb0_mux[] = {
> USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
> };
> +static const unsigned int usb0_ovc_vbus_pins[] = {
> + /* OVC/VBUS */
> + RCAR_GP_PIN(5, 19),
> +};
> +static const unsigned int usb0_ovc_vbus_mux[] = {
> + USB0_OVC_VBUS_MARK,
> +};
Another option would have been to split the existing usb0 group in usb0_pwen
and usb0_ovc. I'm not sure which is better though, I'd just like to know if
you had given it a thought.
Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus
to keep names short ?
> /* - USB1
> ------------------------------------------------------------------- */
> static const unsigned int usb1_pins[] = {
> /* PWEN, OVC */
> @@ -3789,6 +3796,7 @@ static const struct sh_pfc_pin_group pin
> SH_PFC_PIN_GROUP(tpu0_to2),
> SH_PFC_PIN_GROUP(tpu0_to3),
> SH_PFC_PIN_GROUP(usb0),
> + SH_PFC_PIN_GROUP(usb0_ovc_vbus),
> SH_PFC_PIN_GROUP(usb1),
> SH_PFC_PIN_GROUP(usb2),
> VIN_DATA_PIN_GROUP(vin0_data, 24),
> @@ -4134,6 +4142,7 @@ static const char * const tpu0_groups[]
>
> static const char * const usb0_groups[] = {
> "usb0",
> + "usb0_ovc_vbus",
> };
>
> static const char * const usb1_groups[] = {
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 00/21] pinctrl: mvebu: restructure and remove hardcoded addresses from Dove pinctrl
From: Sebastian Hesselbarth @ 2014-01-31 2:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140130202500.GN10864@lunn.ch>
On 01/30/2014 09:25 PM, Andrew Lunn wrote:
> On Thu, Jan 30, 2014 at 07:50:34PM +0100, Sebastian Hesselbarth wrote:
>> On 01/30/2014 07:29 PM, Andrew Lunn wrote:
>>> On Tue, Jan 28, 2014 at 01:39:12AM +0100, Sebastian Hesselbarth wrote:
>>>> This patch set is one required step for Dove to hop into mach-mvebu.
>>>> Until now, pinctrl-dove was hardcoding some registers that do not
>>>> directly belong to MPP core registers. This is not compatible with
>>>> what we want for mach-mvebu.
>>>
>>> I think there might be something wrong here....
>>
>> There _is_ something wrong. I'll have a look at it. For the record,
>> what SoC are you testing with? From the base address, I guess it is
>> Kirkwood?
>
> Yes, Kirkwood. Sorry for not saying.
This time I push a branch before sending out the patches. Also, I
think I'll postpone removal of hardcoded addresses until this is
sorted out. The patch set was growing way to quick and I have to
do this step-by-step for me and everybody else to actually understand ;)
So, at least the MVEBU guys should test the following branch on
their SoCs. Again, I have tested Dove and now confirmed that settings
are still correct. The others are compile-tested.
https://github.com/shesselba/linux-dove.git unstable/mvebu-pinctrl-v3.14_v3
@Thomas, Gregory: Do you think that the above branch will be
restructured enough allow support for orion5x and mv78xx0? I had a
quick look at mach-{orion5x,mv78xx0}/mpp.h and didn't see anything
weird.
Sebastian
^ permalink raw reply
* [PATCH V2] arm64: add DSB after icache flush in __flush_icache_all()
From: Nicolas Pitre @ 2014-01-31 2:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140131001647.GA5525@mudshark.cambridge.arm.com>
On Fri, 31 Jan 2014, Will Deacon wrote:
> Hi Nico,
>
> On Thu, Jan 30, 2014 at 09:42:29PM +0000, Nicolas Pitre wrote:
> > On Thu, 30 Jan 2014, Will Deacon wrote:
> > > On Thu, Jan 30, 2014 at 06:04:43AM +0000, Vinayak Kale wrote:
> > > > Can you please elaborate whether you are referring to lack of memory
> > > > clobber or missing barriers?
> > >
> > > The clobbers. For example:
> > >
> > > arch/arm64/kvm/sys_regs.c:
> > >
> > > /* Make sure noone else changes CSSELR during this! */
> > > local_irq_disable();
> > > /* Put value into CSSELR */
> > > asm volatile("msr csselr_el1, %x0" : : "r" (csselr));
> > > isb();
> > > /* Read result out of CCSIDR */
> > > asm volatile("mrs %0, ccsidr_el1" : "=r" (ccsidr));
> > > local_irq_enable();
> > >
> > > Just about everything can be re-ordered in that block, because the asm
> > > volatile statements don't have "memory" clobbers.
> >
> > I don't think they would be reordered at all with the
> > volatile qualifiers.
>
> Whilst that may be the case in current compilers (i.e. I've not actually
> seen the above sequence get re-ordered), the GCC documentation states that:
>
> Similarly, you can't expect a sequence of volatile asm instructions to remain
> perfectly consecutive. If you want consecutive output, use a single asm. Also,
> GCC performs some optimizations across a volatile asm instruction; GCC does not
> `forget everything' when it encounters a volatile asm instruction the way some
> other compilers do.
>
> so I really think that the "memory" clobbers are needed to ensure strict
> ordering. This matches my understanding from discussions with the compiler
> engineers at ARM.
Well, the key sentence above is: "you can't expect a sequence of
volatile asm instructions to remain perfectly consecutive." That
doesn't mean the "ordering" won't be respected.
If you need a certain ordering, then a volatile is all that you need.
If nothing else should happen in between, then just use a single asm
statement as suggested.
Only if you need ordering with regards to other memory accesses as well
then yes you need a memory clobber in that case.
Nicolas
^ permalink raw reply
* [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Felipe Balbi @ 2014-01-31 2:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGa+x854huSmiWmEv2EgOPUgZKcp3iitNaBvKXt8DiEj8msSVg@mail.gmail.com>
Hi,
On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
> >>
> >> > +config SPI_SUN6I
> >> > + tristate "Allwinner A31 SPI controller"
> >> > + depends on ARCH_SUNXI || COMPILE_TEST
> >> > + select PM_RUNTIME
> >> > + help
> >> > + This enables using the SPI controller on the Allwinner A31 SoCs.
> >> > +
> >>
> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
> >> The usual idiom is that the device starts out powered up (flagged using
> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
> >> compiled in. That way if for some reason people want to avoid runtime
> >> PM they can still use the device.
> >
> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
> > general are defined to pretty much empty functions, how the
> > suspend/resume callbacks are called then? Obviously, we need them to
> > be run, hence why I added the select here, but now I'm seeing a
> > construct like what's following acceptable then?
>
> Even with your 'select', The runtime PM callbacks will never be called
> in the current driver. pm_runtime_enable() doesn't do any runtime PM
> transitions. It just allows transitions to happen when they're
> triggered by _get()/_put()/etc.
>
> > pm_runtime_enable(&pdev->dev);
> > if (!pm_runtime_enabled(&pdev->dev))
> > sun6i_spi_runtime_resume(&pdev->dev);
>
> Similarily here, it's not the pm_runtime_enable that will fail when
> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
> that will fail.
>
> What you want is something like this in ->probe()
>
> sun6i_spi_runtime_resume();
> /* now, device is always activated whether or not runtime PM is enabled */
> pm_runtime_enable();
> pm_runtime_set_active(); /* tells runtime PM core device is
> already active */
shouldn't this be done before pm_runtime_enable() ?
> pm_runtime_get_sync();
>
> This 'get' will increase the usecount, but not actually call the
> callbacks because we told the RPM core that the device was already
> activated with _set_active().
>
> And then, in ->remove(), you'll want
>
> pm_runtime_put();
in ->remove() you actually want a put_sync() right ? You don't want to
schedule anything since you're just about to disable pm_runtime.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140130/eee0da03/attachment-0001.sig>
^ permalink raw reply
* [PATCH 02/03] pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS
From: Magnus Damm @ 2014-01-31 3:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1397857.qyLQY9XTlp@avalon>
Hi Laurent,
On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> Create a new group for the USB0 OVC/VBUS pin by itself. This
>> allows us to monitor PWEN as GPIO on the Lager board.
>>
>> Signed-off-by: Magnus Damm <damm@opensource.se>
>> ---
>>
>> drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
> 10:23:32.000000000
>> +0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
>> static const unsigned int usb0_mux[] = {
>> USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
>> };
>> +static const unsigned int usb0_ovc_vbus_pins[] = {
>> + /* OVC/VBUS */
>> + RCAR_GP_PIN(5, 19),
>> +};
>> +static const unsigned int usb0_ovc_vbus_mux[] = {
>> + USB0_OVC_VBUS_MARK,
>> +};
>
> Another option would have been to split the existing usb0 group in usb0_pwen
> and usb0_ovc. I'm not sure which is better though, I'd just like to know if
> you had given it a thought.
I actually did just that in my first local attempt, but I decided not
to since it will only cause potential breakage.
> Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus
> to keep names short ?
Is there any particular reason why you want shorter names?
>From my side, I prefer to keep the names in sync with the data sheet.
In this particular case it is a shared pin so OVC is used for Host
while VBUS is used for gadget, so if you're proposing to ditch VBUS
then this feels somewhat inconsistent with the current gadget use
case. =)
Thanks,
/ magnus
^ permalink raw reply
* [PATCH 03/03] ARM: shmobile: Lager USB0 cable detection workaround
From: Magnus Damm @ 2014-01-31 3:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1682649.H5bjffAvcD@avalon>
Hi Laurent,
[CC Morimoto-san, the author of the USBHS driver]
On Fri, Jan 31, 2014 at 10:14 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Thursday 30 January 2014 08:10:29 Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> Add Lager board code to check the PWEN GPIO signal and refuse to
>> allow probe of the USBHS driver in case of DIP misconfiguration.
>>
>> For correct operation Lager DIP switches SW5 and SW6 shall be
>> configured in 2-3 position to enable USB Function support.
>>
>> If the DIP switch is configured incorrectly then the user can
>> simply adjust the hardware and either reboot or use the bind interface
>> to try to probe again:
>>
>> # echo renesas_usbhs > /sys/bus/platform/drivers/renesas_usbhs/bind
>
> Our of curiosity, and I know you will love the question, have you thought
> about how to implement this on multiplatform kernels without a board file ?
> :-)
Thanks for asking. =)
Actually I've been giving this some thought already. And it happens to
be that the sister board "Koelsch" has similar but slightly improved
hardware.
In the Lager case we can only support USB Function properly and
safely, and to avoid shooting ourselves in the foot we need a check
like this patch implements. So on Lager RCAR_GP_PIN(5, 18) needs to be
high for proper USB Function operation. If not we give up.
In case of Koelsch a MAX3355E chip is hooked up to handle dual
function or OTG. So on that platform we can use either Function or
Host. To figure out which cable is hooked up we can check the ID pin
of the USB connector through the MAX3355E ID_OUT signal.
In general with a USB connector (and the MAX3355E chip) there are
standard signal levels for ID pin detection. The ID signal for the USB
micro-AB connector will be High in case of Function cable (pull-up,
left open), and Low in case of Host (tied to GND).
So, to answer your question about how to support this without
board-specific code written in C, I believe we need to add
abstractions to support the MAX3355E chip and/or external ID pin
somehow. Perhaps there is almost nothing to abstract there?
Regarding how to support Lager then I think we can use the same code
as Koelsch because the ID pin signal is following the same signal
level. So perhaps it is enough to add support for the USBHS driver to
handle the ID pin as GPIO? If so, then we can use the same style for
both Lager and Koelsch, but on Lager we also somehow need to tell the
the USBHS driver that only Function is OK.
> This might be one of the valid cases where we won't be able to do without a
> board file. The callback functions in platform data, however, probably need to
> go.
I think it is possible to get rid of the board file. And especially if
we can reuse the code for multiple boards or SoC then this starts
making sense - even to me! =)
Thanks,
/ magnus
^ permalink raw reply
* [PATCH V3 6/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform code to phy driver
From: Pratyush Anand @ 2014-01-31 4:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201401301421.00990.arnd@arndb.de>
On Thu, Jan 30, 2014 at 09:21:00PM +0800, Arnd Bergmann wrote:
> On Thursday 30 January 2014, Mohit Kumar wrote:
> >
> > diff --git a/Documentation/devicetree/bindings/phy/spear13xx-miphy.txt b/Documentation/devicetree/bindings/phy/spear13xx-miphy.txt
> > new file mode 100644
> > index 0000000..208b37d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/spear13xx-miphy.txt
> > @@ -0,0 +1,8 @@
> > +Required properties:
> > +- compatible : should be "st,spear1340-sata-pcie-phy".
>
> Just for confirmation: This phy is by design only capable of driving
> sata or pcie, but nothing else if reused in a different SoC, right?
>
> If the phy is actually more generic than that, I'd suggest changing
> the name, otherwise it's ok.
OK, we will give a generic name as it can be used for sata/pcie/usb3.0.
Although, phy register definition may vary from version to version,
but that can be managed,as and when support of new soc is added.
>
> > +- reg : offset and length of the PHY register set.
> > +- misc: phandle for the syscon node to access misc registers
> > +- #phy-cells : from the generic PHY bindings, must be 2.
> > + - 1st arg: phandle to the phy node.
> > + - 2nd arg: 0 if phy (in 1st arg) is to be used for sata else 1.
> > + - 3rd arg: Instance id of the phy (in 1st arg).
>
> I would count "arg" differently: There are three cells, and the first
> cell is the phandle, while the second and third cells contain the first
> and second argument.
Ok..will modify accordingly.
>
> The third cell seems redundant, more on that below.
>
> > + ahci0: ahci at b1000000 {
> > compatible = "snps,spear-ahci";
> > reg = <0xb1000000 0x10000>;
> > interrupts = <0 68 0x4>;
> > + phys = <&miphy0 0 0>;
> > + phy-names = "ahci-phy";
> > status = "disabled";
> > };
> >
> > - ahci at b1800000 {
> > + ahci1: ahci at b1800000 {
> > compatible = "snps,spear-ahci";
> > reg = <0xb1800000 0x10000>;
> > interrupts = <0 69 0x4>;
> > + phys = <&miphy1 0 1>;
> > + phy-names = "ahci-phy";
> > status = "disabled";
> > };
> >
> > - ahci at b4000000 {
> > + ahci2: ahci at b4000000 {
> > compatible = "snps,spear-ahci";
> > reg = <0xb4000000 0x10000>;
> > interrupts = <0 70 0x4>;
> > + phys = <&miphy2 0 2>;
> > + phy-names = "ahci-phy";
> > status = "disabled";
> > };
>
> In each case, the number of the phy 'miphyX' is identical to the
> third cell, and I suspect this is by design. In the driver, the
> 'id' field is set in the xlate function, but I could not find any
> place where it actually gets used, so unless you know that it's
> needed, I'd suggest simply removing it.
It has not been used in this patch, as SATA support is currently only
for SPEAr1340, where we have only one instance.
Will be using it in PCIe for SPEAr1310 where 3 instances are present.
>
> Even if you need it, it may be better to have the instance encoded
> in the phy node itself, since it's a property of the phy hardware
> (e.g. if you have to pass the number into a generic register that
> is global to all phys.
Ok..ll do that.
>
> Alternatively, you could have a different representation, where you
> have a single DT device node representing all three PHYs, with
> "reg = <0xeb800000 0xc000>;" In that case, all sata devices would
> point to the same phy node and pass the instance id so the phy
> driver can operated the correct register set.
Instance ID is mainly needed to manipulate wrapper register present
within SPEAr13xx misc space. We have a single register in misc space
having bit fields controlling all 3 phys, and there we need this id.
>
> > +static int spear1340_sata_miphy_init(struct spear13xx_phy_priv *phypriv)
> > +{
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_SATA_CFG,
> > + SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
> > + SPEAR1340_PCIE_MIPHY_CFG_MASK,
> > + SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
> > + /* Switch on sata power domain */
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PCM_CFG,
> > + SPEAR1340_PCM_CFG_SATA_POWER_EN,
> > + SPEAR1340_PCM_CFG_SATA_POWER_EN);
> > + msleep(20);
> > + /* Disable PCIE SATA Controller reset */
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PERIP1_SW_RST,
> > + SPEAR1340_PERIP1_SW_RST_SATA, 0);
> > + msleep(20);
> > +
> > + return 0;
> > +}
>
> I guess some of the parts above can eventually get moved into other
> drivers (reset controller, power domains) that get called directly
> by the SATA driver (e.g. though reset_device()). Since that won't
> impact the PHY binding, it seems fine to leave it here for now.
thanks :)
Regards
Pratyush
>
> Arnd
^ permalink raw reply
* recommended action for bootloaders regarding modifying device-tree nodes
From: Tim Harvey @ 2014-01-31 4:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140130204558.GC29184@titan.lakedaemon.net>
On Thu, Jan 30, 2014 at 12:45 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> Hi Tim,
>
> On Thu, Jan 30, 2014 at 01:11:18AM -0800, Tim Harvey wrote:
>> My approach has been to define a per-baseboard device-tree in Linux
>> for a 'fully loaded' board, then remove nodes which the EEPROM claims
>> are not present in the bootloader before it passes the DTB to the
>> kernel. I do this by defining aliases in the device-tree for the
>> peripherals that are 'optional' so that the bootloader itself does not
>> need to know the details about how the device is connected.
>
> This is more of a process question: Is there any information captured
> in your EEPROM that can't be represented in the dtb? iow, at the point
> when you write the EEPROM, why not write the dtb to it as configured?
>
> You could have pre-configured dtsi fragments for each config option, and
> then dynamically create the board dts from the order.
>
> I only ask because it would solve the problem below. However, there's a
> lot more to changing a manufacturing process than meets the eye. :)
>
our eeprom config section is only 40 bytes. It contains a SKU string,
mac addrs, and some bitwise fields for the various optional components
that we can subload.
>> Is it more appropriate for the bootloader to 'remove' nodes for
>> devices that are not physically present or should I be setting their
>> status property to 'disabled' instead? I'm not clear if either option
>> really has any pros or cons.
>
> That depends on how you have it structured. Is it a valid dtb?
> Meaning, do you have four nodes all at the same register address?
> Perhaps you could provide an example dts?
yes its a valid dtb - it is just the superset of everything the
baseboard (ie schematic design) can support.
A good example is a custom SKU of a baseboard with ethernet subloaded.
If the EEPROM says there is no ethernet mac or phy, I would want to
remove or disable the ethernet node from the devicetree.
Another example would be a node for 'gpio-pps' (GPIO based
pulse-per-second) support. A baseboard design that has a GPS with its
PPS signal tied to a GPIO would define this in the device-tree, but if
the EEPROM says the GPS isn't loaded, I would want to remove or
disable the gps-pps node.
Tim
>
> thx,
>
> Jason.
>
>> Tim Harvey - Principal Software Engineer
>> Gateworks Corporation
>
> btw - one of my first embedded projects was on one of your boards. An
> ixp425 with 4 mini-pci slots.
>
^ permalink raw reply
* recommended action for bootloaders regarding modifying device-tree nodes
From: Tim Harvey @ 2014-01-31 4:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140130211512.GD13372@obsidianresearch.com>
On Thu, Jan 30, 2014 at 1:15 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
>
> On Thu, Jan 30, 2014 at 03:45:58PM -0500, Jason Cooper wrote:
>
> > This is more of a process question: Is there any information captured
> > in your EEPROM that can't be represented in the dtb? iow, at the point
> > when you write the EEPROM, why not write the dtb to it as configured?
>
> I can share what we do here.. In our systems the serial EEPROM is only
> 256 bytes, so storing things in DT format would be challenging.
>
> What we do is have a master DTB that has the union of all our
> configurations. The boot process has a very simple bit of code that
> runs down the DTB in binary format and replaces entire
> OF_DT_BEGIN_NODE->OF_DT_END_NODE regions with OF_DT_NOP.
>
> The NOP approach is very simple, no other changes (eg offset
> recalculation) needs to be done to the DT, so we can do this process
> with a very small code footprint and without libfdt.
>
> Choosing which sections to drop is done with some combination of
> hardwired code and searching for specific property patterns. There are
> also a few places where placeholder sections are directly fixed up, eg
> a mac address is written into a placeholder of 0s, etc.
>
> So an example might be
>
> optional_peripheral at 10000 {
> orc,board-style = <1>;
> [..]
> }
>
> Eg The board-style number comes from the EEPROM and if board-style !=
> 1 then the entire stanza is replaced with NOP.
>
> Jason
Jason,
Sounds pretty much like what we are doing. I am using u-boot and my
current code looks like this:
/*
* Peripheral Config:
* remove nodes by alias path if EEPROM config tells us the
* peripheral is not loaded on the board.
*/
if (!test_bit(EECONFIG_ETH0, info->config))
fdt_del_node_and_alias(blob, "ethernet0");
if (!test_bit(EECONFIG_ETH1, info->config))
fdt_del_node_and_alias(blob, "ethernet1");
if (!test_bit(EECONFIG_HDMI_OUT, info->config))
fdt_del_node_and_alias(blob, "hdmi_out");
if (!test_bit(EECONFIG_SATA, info->config))
fdt_del_node_and_alias(blob, "ahci0");
if (!test_bit(EECONFIG_PCIE, info->config))
fdt_del_node_and_alias(blob, "pcie");
if (!test_bit(EECONFIG_SSI0, info->config))
fdt_del_node_and_alias(blob, "ssi0");
if (!test_bit(EECONFIG_SSI1, info->config))
fdt_del_node_and_alias(blob, "ssi1");
...
I've submitted my code to u-boot and have been asked if its more
appropriate to remove nodes as I'm doing above or to mark them as
'disabled'. From what I can tell there really isn't a rule or
recommendation for this so I think I'll keep doing what I'm doing
above.
Thanks!
Tim
^ permalink raw reply
* [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Kevin Hilman @ 2014-01-31 5:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140131022954.GB8163@saruman.home>
On Thu, Jan 30, 2014 at 6:29 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
>> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
>> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
>> >>
>> >> > +config SPI_SUN6I
>> >> > + tristate "Allwinner A31 SPI controller"
>> >> > + depends on ARCH_SUNXI || COMPILE_TEST
>> >> > + select PM_RUNTIME
>> >> > + help
>> >> > + This enables using the SPI controller on the Allwinner A31 SoCs.
>> >> > +
>> >>
>> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
>> >> The usual idiom is that the device starts out powered up (flagged using
>> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
>> >> compiled in. That way if for some reason people want to avoid runtime
>> >> PM they can still use the device.
>> >
>> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
>> > general are defined to pretty much empty functions, how the
>> > suspend/resume callbacks are called then? Obviously, we need them to
>> > be run, hence why I added the select here, but now I'm seeing a
>> > construct like what's following acceptable then?
>>
>> Even with your 'select', The runtime PM callbacks will never be called
>> in the current driver. pm_runtime_enable() doesn't do any runtime PM
>> transitions. It just allows transitions to happen when they're
>> triggered by _get()/_put()/etc.
>>
>> > pm_runtime_enable(&pdev->dev);
>> > if (!pm_runtime_enabled(&pdev->dev))
>> > sun6i_spi_runtime_resume(&pdev->dev);
>>
>> Similarily here, it's not the pm_runtime_enable that will fail when
>> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
>> that will fail.
>>
>> What you want is something like this in ->probe()
>>
>> sun6i_spi_runtime_resume();
>> /* now, device is always activated whether or not runtime PM is enabled */
>> pm_runtime_enable();
>> pm_runtime_set_active(); /* tells runtime PM core device is
>> already active */
>
> shouldn't this be done before pm_runtime_enable() ?
hmm, possibly yes. I was doing this from the top of my head without
looking to closely at the code.
>> pm_runtime_get_sync();
>>
>> This 'get' will increase the usecount, but not actually call the
>> callbacks because we told the RPM core that the device was already
>> activated with _set_active().
>>
>> And then, in ->remove(), you'll want
>>
>> pm_runtime_put();
>
> in ->remove() you actually want a put_sync() right ? You don't want to
> schedule anything since you're just about to disable pm_runtime.
Yes, you're correct.
Thanks for the corrections.
Kevin
^ permalink raw reply
* [PATCH] ARM: integrator: restore static map on the CP
From: Kevin Hilman @ 2014-01-31 6:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390568668-32270-1-git-send-email-linus.walleij@linaro.org>
Linus Walleij <linus.walleij@linaro.org> writes:
> Commit 78d1632183454dba46ca8295484a5e7603acdc18 deleted the
> static mappings of the core modules, but this static map is
> still needed on the Integrator/CP (not the Integrator/AP).
>
> Restore the static map on the Integrator/CP.
>
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ARM SoC folks: please apply this fix directly to ARM SoC
> if you're OK with it.
Queuing for fixes.
Kevin
^ permalink raw reply
* [PATCH v2] ARM: iop32x: fix power off handling for the EM7210 board
From: Kevin Hilman @ 2014-01-31 6:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391005215-8520-1-git-send-email-linus.walleij@linaro.org>
Linus Walleij <linus.walleij@linaro.org> writes:
> This board was missed when converting all the others to proper
> abstracted GPIO handling. Fix it up the right way by requesting
> and driving GPIO line 0 high through gpiolib to power off the
> machine.
>
> Cc: Arnaud Patard <arnaud.patard@rtp-net.org>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Queueing for fixes (w/ack from Arnd.)
Kevin
^ permalink raw reply
* [PATCH v5 4/4] [media] exynos-scaler: Add DT bindings for SCALER driver
From: Shaik Ameer Basha @ 2014-01-31 6:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E29051.3070906@samsung.com>
Hi Tomasz,
Thanks for the review.
Will consider all your comments in the next version of patch series.
Regards,
Shaik Ameer Basha
On Sat, Jan 25, 2014 at 1:09 AM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Shaik,
>
>
> On 09.01.2014 04:28, Shaik Ameer Basha wrote:
>>
>> This patch adds the DT binding documentation for the
>> Exynos5420/5410 based SCALER device driver.
>>
>> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
>> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>> .../devicetree/bindings/media/exynos5-scaler.txt | 22
>> ++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/exynos5-scaler.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> new file mode 100644
>> index 0000000..9328e7d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> @@ -0,0 +1,22 @@
>> +* Samsung Exynos5 SCALER device
>> +
>> +SCALER is used for scaling, blending, color fill and color space
>> +conversion on EXYNOS[5420/5410] SoCs.
>> +
>> +Required properties:
>> +- compatible: should be "samsung,exynos5420-scaler" or
>> + "samsung,exynos5410-scaler"
>> +- reg: should contain SCALER physical address location and length
>> +- interrupts: should contain SCALER interrupt number
>
>
> s/number/specifier/
>
>
>> +- clocks: should contain the SCALER clock specifier, from the
>> + common clock bindings
>
>
> s/specifier/phandle and specifier pair for each clock listed in clock-names
> property/
>
> s/from/according to/
>
>
>> +- clock-names: should be "scaler"
>
>
> should contain exactly one entry:
> - "scaler" - IP bus clock.
>
> Also this patch should be first in the series to let the driver added in
> further patches use already present bindings.
>
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
>
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-01-31 6:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140128031324.GH10628@intel.com>
Hi Vinod,
On Tue, Jan 28, 2014 at 8:43 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Mon, Jan 27, 2014 at 06:42:36PM +0530, Srikanth Thokala wrote:
>> Hi Lars/Vinod,
>> >> The question here i think would be waht this device supports? Is the hardware
>> >> capable of doing interleaved transfers, then would make sense.
>> >
>> > The hardware does 2D transfers. The parameters for a transfer are height,
>> > width and stride. That's only a subset of what interleaved transfers can be
>> > (xt->num_frames must be one for 2d transfers). But if I remember correctly
>> > there has been some discussion on this in the past and the result of that
>> > discussion was that using interleaved transfers for 2D transfers is
>> > preferred over adding a custom API for 2D transfers.
>>
>> I went through the prep_interleaved_dma API and I see only one descriptor
>> is prepared per API call (i.e. per frame). As our IP supports upto 16 frame
>> buffers (can be more in future), isn't it less efficient compared to the
>> prep_slave_sg where we get a single sg list and can prepare all the descriptors
>> (of non-contiguous buffers) in one go? Correct me, if am wrong and let me
>> know your opinions.
> Well the descriptor maybe one, but that can represent multiple frames, for
> example 16 as in your case. Can you read up the documentation of how multiple
> frames are passed. Pls see include/linux/dmaengine.h
>
> /**
> * Interleaved Transfer Request
> * ----------------------------
> * A chunk is collection of contiguous bytes to be transfered.
> * The gap(in bytes) between two chunks is called inter-chunk-gap(ICG).
> * ICGs may or maynot change between chunks.
> * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
> * that when repeated an integral number of times, specifies the transfer.
> * A transfer template is specification of a Frame, the number of times
> * it is to be repeated and other per-transfer attributes.
> *
> * Practically, a client driver would have ready a template for each
> * type of transfer it is going to need during its lifetime and
> * set only 'src_start' and 'dst_start' before submitting the requests.
> *
> *
> * | Frame-1 | Frame-2 | ~ | Frame-'numf' |
> * |====....==.===...=...|====....==.===...=...| ~ |====....==.===...=...|
> *
> * == Chunk size
> * ... ICG
> */
Yes, it can handle multiple frames specified by 'numf' each of size
'frame_size * sgl[0].size'.
But, I see it only works if all the frames' memory is contiguous and
in this case we
can just increment 'src_start' by the total frame size 'numf' number
of times to fill in
for each HW descriptor (each frame is one HW descriptor). So, there
is no issue when the
memory is contiguous. If the frames are non contiguous, we have to
call this API for each
frame (hence for each descriptor), as the src_start for each frame is
different. Is it correct?
FYI: This hardware has an inbuilt Scatter-Gather engine.
Srikanth
>
> --
> ~Vinod
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-01-31 6:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+mB=1Jwjn+KSO9yFr=LNVjk3khf0b-LLQamYf+nnSF4bODGPA@mail.gmail.com>
Hi Vinod,
On Mon, Jan 27, 2014 at 4:36 PM, Srikanth Thokala <sthokal@xilinx.com> wrote:
> Hi Vinod,
>
> On Sun, Jan 26, 2014 at 7:29 PM, Vinod Koul <vinod.koul@intel.com> wrote:
>> On Fri, Jan 24, 2014 at 02:24:27PM +0100, Lars-Peter Clausen wrote:
>>> On 01/24/2014 12:16 PM, Srikanth Thokala wrote:
>>> > Hi Lars,
>>> >
>>> > On Thu, Jan 23, 2014 at 4:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> >> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
>>> >> [...]
>>> >>> +/**
>>> >>> + * xilinx_vdma_device_control - Configure DMA channel of the device
>>> >>> + * @dchan: DMA Channel pointer
>>> >>> + * @cmd: DMA control command
>>> >>> + * @arg: Channel configuration
>>> >>> + *
>>> >>> + * Return: '0' on success and failure value on error
>>> >>> + */
>>> >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
>>> >>> + enum dma_ctrl_cmd cmd, unsigned long arg)
>>> >>> +{
>>> >>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>>> >>> +
>>> >>> + switch (cmd) {
>>> >>> + case DMA_TERMINATE_ALL:
>>> >>> + xilinx_vdma_terminate_all(chan);
>>> >>> + return 0;
>>> >>> + case DMA_SLAVE_CONFIG:
>>> >>> + return xilinx_vdma_slave_config(chan,
>>> >>> + (struct xilinx_vdma_config *)arg);
>>> >>
>>> >> You really shouldn't be overloading the generic API with your own semantics.
>>> >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
>>> >
>>> > Ok. The driver needs few additional configuration from the slave
>>> > device like Vertical
>>> > Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you
>>> > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
>>> > defined for Freescale drivers?
>>>
>>> In my opinion it is not a good idea to have driver implement a generic API,
>>> but at the same time let the driver have custom semantics for those API
>>> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
>>> values passed to gpio_set_value instead of 0 and 1. It completely defeats
>>> the purpose of a generic API, namely that you are able to write generic code
>>> that makes use of the API without having to know about which implementation
>>> API it is talking to. The dmaengine framework provides the
>>> dmaengine_prep_interleaved_dma() function to setup two dimensional
>>> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
>>
>> The question here i think would be waht this device supports? Is the hardware
>> capable of doing interleaved transfers, then would make sense.
>>
>> While we do try to get users use dma_slave_config, but there will always be
>> someone who have specfic params. If we can generalize then we might want to add
>> to the dma_slave_config as well
>
> There are many configuration parameters which are specific to IP and I
> would like to
> give an overview of some of parameteres here:
>
> 1) Park Mode ('cfg->park'): In Park mode, engine will park on frame
> referenced by
> 'cfg->park_frm', so user will have control on each frame in this mode.
>
> 2) Interrupt Coalesce ('cfg->coalesce'): Used for setting interrupt
> threshold. This value
> determines the number of frame buffers to process. To use this feature,
> 'cfg->frm_cnt_en' should be set.
>
> 3) Frame Synchronization Source ('cfg->ext_fsync'): Can be an
> external/internal frame
> synchronization source. Used to synchronize one channel (MM2S/S2MM) with
> another (S2MM/MM2S) channel.
>
> 4) Genlock Synchronization ('cfg->genlock'): Used to avoid mismatch rate between
> master and slave. In master mode (cfg->master), frames are not dropped and
> slave can drop frames to adjust to master frame rate.
>
> And in future, this Engine being a soft IP, we could expect some more additional
> parameters. Isn't a good idea to have a private member in dma_slave_config for
> sharing additional configuration between slave device and dma engine? Or a new
> dma_ctrl_cmd like FSLDMA_EXTERNAL_START?
Ping?
>
> Srikanth
>
>>
>> --
>> ~Vinod
>> --/EX
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* [PATCH v2 1/2] at91: gpio: use gpiolib API to mark a GPIO used as an IRQ
From: Linus Walleij @ 2014-01-31 7:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390473478-5591-2-git-send-email-jjhiblot@traphandler.com>
On Thu, Jan 23, 2014 at 11:37 AM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:
> When an IRQ is started on a GPIO line, mark this GPIO as IRQ in
> the gpiolib so we can keep track of the usage centrally.
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Nicolas, are you queueing this patch?
I guess this custom GPIO implementation is going away with
the transition to pinctrl else I'd requested that it be moved to
drivers/gpio...
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2 2/2] at91: pinctrl: use gpiolib API to mark a GPIO used as an IRQ
From: Linus Walleij @ 2014-01-31 7:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390473478-5591-3-git-send-email-jjhiblot@traphandler.com>
On Thu, Jan 23, 2014 at 11:37 AM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:
> When an IRQ is started on a GPIO line, mark this GPIO as IRQ in
> the gpiolib so we can keep track of the usage centrally.
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
Thanks!
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.
From: Linus Walleij @ 2014-01-31 8:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACh+v5Ot4ZwJrE_nJyJn-h-ZNrHfCk60c6KH_ErnFkD+u7atng@mail.gmail.com>
On Thu, Jan 23, 2014 at 2:16 PM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:
> I've been thinking hard on the gpio ownership and I'd like your
> opinion on the following assumptions:
>
> - a gpio used as an interrupt is always a gpio configured as an input.
> (corollary : a gpio used for interrupt cannot not be an ouput)
That is the assumption made by the core. Until the day someone
comes up with a weird usecase...
> - a gpio used as an input or interrupt only, could be safely accessed
> by multiple users.
Multiple consumers is the term we would use. But yes.
> - a gpio used as an output should be used as such by only one user.
> Still other users should be allowed to read its value.
Sounds reasonable.
> If those assumptions are true, I believe they can lead to a new
> exclusion scheme:
> - ouput is mutually exclusive with interrupt but not with input
> - only 1 ouput user at a time.
Logical conclusion from the above yes.
> It would do away with our sharing issue and more (chained interrupt
> handlers on gpio interrupt, read/interrupt access through sysfs to a
> gpio requested by a driver)
>
> And I believe that most of the infrastructure is in place to implement this :
> - direction flags in gpio_request_one
> - gpio_lock_as_irq/gpio_unlock_as_irq
So I think what I need to see is a proposed patch, whether it will
hit a particular driver or the gpiolib core, but either would probably
be interesting.
Pls keep linux-gpio and Alexandre on CC for this discussion.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Maxime Ripard @ 2014-01-31 8:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGa+x854huSmiWmEv2EgOPUgZKcp3iitNaBvKXt8DiEj8msSVg@mail.gmail.com>
Hi Kevin,
On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
> >>
> >> > +config SPI_SUN6I
> >> > + tristate "Allwinner A31 SPI controller"
> >> > + depends on ARCH_SUNXI || COMPILE_TEST
> >> > + select PM_RUNTIME
> >> > + help
> >> > + This enables using the SPI controller on the Allwinner A31 SoCs.
> >> > +
> >>
> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
> >> The usual idiom is that the device starts out powered up (flagged using
> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
> >> compiled in. That way if for some reason people want to avoid runtime
> >> PM they can still use the device.
> >
> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
> > general are defined to pretty much empty functions, how the
> > suspend/resume callbacks are called then? Obviously, we need them to
> > be run, hence why I added the select here, but now I'm seeing a
> > construct like what's following acceptable then?
>
> Even with your 'select', The runtime PM callbacks will never be called
> in the current driver. pm_runtime_enable() doesn't do any runtime PM
> transitions. It just allows transitions to happen when they're
> triggered by _get()/_put()/etc.
Actually, pm_runtime_get_sync is called by the SPI framework whenever
the device needs to be used. And pm_runtime_put whenever it's not used
anymore, so the callbacks are actually called.
>
> > pm_runtime_enable(&pdev->dev);
> > if (!pm_runtime_enabled(&pdev->dev))
> > sun6i_spi_runtime_resume(&pdev->dev);
>
> Similarily here, it's not the pm_runtime_enable that will fail when
> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
> that will fail.
In the case where pm_runtime is disabled, pm_runtime_enabled will only
return false, and hence the resume callback will be called. get_sync
will fail too when the framework will call it, but since the device is
already initialized, it's fine I guess.
> What you want is something like this in ->probe()
>
> sun6i_spi_runtime_resume();
> /* now, device is always activated whether or not runtime PM is enabled */
> pm_runtime_enable();
> pm_runtime_set_active(); /* tells runtime PM core device is
> already active */
> pm_runtime_get_sync();
>
> This 'get' will increase the usecount, but not actually call the
> callbacks because we told the RPM core that the device was already
> activated with _set_active().
>
> And then, in ->remove(), you'll want
>
> pm_runtime_put();
> pm_runtime_disable();
>
> And if runtime PM is not enabled in the kernel, then the device will
> be left on (which is kinda what you want if you didn't build runtime
> PM into the kernel.)
Yes, but that also mean that the device is actually on after the
probe, even if it's never going to be used. From what I got reading
the pm_runtime code, the suspend callback is called only whenever you
call _put, so the device will be suspended only after it's been used
the first time, right?
Wouldn't it be better if it was suspended by default, and just waken
up whenever the framework needs it?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140131/489f461a/attachment.sig>
^ permalink raw reply
* [PATCH v2 RESEND] vt8500: pinctrl: Change devicetree data parsing
From: Linus Walleij @ 2014-01-31 8:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390467453-8841-1-git-send-email-linux@prisktech.co.nz>
On Thu, Jan 23, 2014 at 9:57 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
> Due to an assumption in the VT8500 pinctrl driver, the value passed
> from devicetree for 'wm,pull' was not explicitly translated before
> being passed to pinconf.
>
> Since v3.10, changes to 'enum pin_config_param', PIN_CONFIG_BIAS_PULL_(UP/DOWN)
> no longer map 1-to-1 with the expected values in devicetree.
>
> This patch adds a small translation between the devicetree values (0..2)
> and the enum pin_config_param equivalent values.
>
> Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> ---
> v2:
Patch applied to fixes and tagged for stable v3.10+
Yours,
Linus Walleij
^ permalink raw reply
* iommu/arm-smmu: Regression (sleeping function called from invalid context)
From: Andreas Herrmann @ 2014-01-31 8:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140130235552.GL13543@alberich>
On Fri, Jan 31, 2014 at 12:55:52AM +0100, Andreas Herrmann wrote:
> Hi Will,
>
> Seems that commit a44a9791e778d9ccda50d5534028ed4057a9a45b
> (iommu/arm-smmu: use mutex instead of spinlock for locking page tables)
> introduced a regression.
>
> At least I've hit
>
> BUG: scheduling while atomic: ksoftirqd/0/3/0x00000100
>...
> BUG: sleeping function called from invalid context at mm/page_alloc.c:2679
> in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.13.0-00016-g6e90346 #413
> [<c0014740>] (unwind_backtrace+0x0/0xf8) from [<c00115b0>] (show_stack+0x10/0x14)
> [<c00115b0>] (show_stack+0x10/0x14) from [<c057ea24>] (dump_stack+0x74/0xa8)
> [<c057ea24>] (dump_stack+0x74/0xa8) from [<c00acc1c>] (__alloc_pages_nodemask+0x174/0x930)
> [<c00acc1c>] (__alloc_pages_nodemask+0x174/0x930) from [<c042a250>] (arm_smmu_handle_mapping+0x470/0x66c)
> [<c042a250>] (arm_smmu_handle_mapping+0x470/0x66c) from [<c0428e74>] (iommu_map+0xf0/0x148)
> [<c0428e74>] (iommu_map+0xf0/0x148) from [<c001935c>] (__map_sg_chunk+0x198/0x2d4)
>...
> Maybe that was the reason why the offending commit was introduced(?).
>
> I think with the current code "atomic allocations" should be used when
> IO page tables are created. With below patch I've not triggered above
> errors.
I think allocating memory with GFP_KERNEL in this dma-mapping path
doesn't seem to be a good idea. What if the DMA operation for which we
modify IO page tables was triggered to free pages (page cache, swap)?
Andreas
^ permalink raw reply
* [PATCH V3 3/8] SPEAr13xx: defconfig: Update
From: Mohit KUMAR DCG @ 2014-01-31 8:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201401301402.01842.arnd@arndb.de>
Hello Arnd,
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: Thursday, January 30, 2014 6:32 PM
> To: Mohit KUMAR DCG
> Cc: Pratyush ANAND; spear-devel; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH V3 3/8] SPEAr13xx: defconfig: Update
>
> On Thursday 30 January 2014, Mohit Kumar wrote:
> > Enable EABI, OEABI, VFP and NFS configs in default configuration file
> > for SPEAr13xx.
>
> Are you sure about OABI_COMPAT? That seems unusual.
- yes, this option is not required
>
> Also, please add all the options you need to multi_v7_defconfig and ensure
> that this configuration works with your hardware as well.
- OK, will do this and test with SPEAr1310.
Thanks
Mohit
^ permalink raw reply
* [PATCH v2 1/2] at91: gpio: use gpiolib API to mark a GPIO used as an IRQ
From: Nicolas Ferre @ 2014-01-31 8:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdZ4LxkRERfD-prZvJSOZyomgf8v8P+vLhb_+hrSkL8rzg@mail.gmail.com>
On 31/01/2014 08:55, Linus Walleij :
> On Thu, Jan 23, 2014 at 11:37 AM, Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>
>> When an IRQ is started on a GPIO line, mark this GPIO as IRQ in
>> the gpiolib so we can keep track of the usage centrally.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Nicolas, are you queueing this patch?
Yes, I will.
> I guess this custom GPIO implementation is going away with
> the transition to pinctrl else I'd requested that it be moved to
> drivers/gpio...
Absolutely. But I suspect it will still be around for some time...
Thanks for your review. Bye,
--
Nicolas Ferre
^ permalink raw reply
* iommu/arm-smmu: Regression (sleeping function called from invalid context)
From: Andreas Herrmann @ 2014-01-31 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140131084623.GN13543@alberich>
On Fri, Jan 31, 2014 at 09:46:23AM +0100, Andreas Herrmann wrote:
> On Fri, Jan 31, 2014 at 12:55:52AM +0100, Andreas Herrmann wrote:
> > Hi Will,
> >
> > Seems that commit a44a9791e778d9ccda50d5534028ed4057a9a45b
> > (iommu/arm-smmu: use mutex instead of spinlock for locking page tables)
> > introduced a regression.
> >
> > At least I've hit
> >
> > BUG: scheduling while atomic: ksoftirqd/0/3/0x00000100
> >...
>
> > BUG: sleeping function called from invalid context at mm/page_alloc.c:2679
> > in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.13.0-00016-g6e90346 #413
> > [<c0014740>] (unwind_backtrace+0x0/0xf8) from [<c00115b0>] (show_stack+0x10/0x14)
> > [<c00115b0>] (show_stack+0x10/0x14) from [<c057ea24>] (dump_stack+0x74/0xa8)
> > [<c057ea24>] (dump_stack+0x74/0xa8) from [<c00acc1c>] (__alloc_pages_nodemask+0x174/0x930)
> > [<c00acc1c>] (__alloc_pages_nodemask+0x174/0x930) from [<c042a250>] (arm_smmu_handle_mapping+0x470/0x66c)
> > [<c042a250>] (arm_smmu_handle_mapping+0x470/0x66c) from [<c0428e74>] (iommu_map+0xf0/0x148)
> > [<c0428e74>] (iommu_map+0xf0/0x148) from [<c001935c>] (__map_sg_chunk+0x198/0x2d4)
> >...
>
> > Maybe that was the reason why the offending commit was introduced(?).
> >
> > I think with the current code "atomic allocations" should be used when
> > IO page tables are created. With below patch I've not triggered above
> > errors.
>
> I think allocating memory with GFP_KERNEL in this dma-mapping path
> doesn't seem to be a good idea. What if the DMA operation for which we
> modify IO page tables was triggered to free pages (page cache, swap)?
I mean in case we run out of memory wouldn't we worsen the situation
by triggering additional IO (and thus DMA)? Whereas when we let the
mapping fail, the OS "just might have to wait a little bit" until
other DMA activities are completed, pages unmapped and iova freed. The
freed resources instantly can be used for further DMA activities.
Hmm, but maybe I need to rethink this (and look more closely into
page_alloc.c).
Andreas
^ permalink raw reply
* [PATCH resend 1/2] arm64: defer reloading a task's FPSIMD state to userland resume
From: Ard Biesheuvel @ 2014-01-31 10:13 UTC (permalink / raw)
To: linux-arm-kernel
If a task gets scheduled out and back in again and nothing has touched
its FPSIMD state in the mean time, there is really no reason to reload
it from memory. Similarly, repeated calls to kernel_neon_begin() and
kernel_neon_end() will preserve and restore the FPSIMD state every time.
This patch defers the FPSIMD state restore to the last possible moment,
i.e., right before the task re-enters userland. If a task does not enter
userland at all (for any reason), the existing FPSIMD state is preserved
and may be reused by the owning task if it gets scheduled in again on the
same CPU.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/fpsimd.h | 3 ++
arch/arm64/include/asm/thread_info.h | 4 +-
arch/arm64/kernel/entry.S | 2 +-
arch/arm64/kernel/fpsimd.c | 79 +++++++++++++++++++++++++++++++-----
arch/arm64/kernel/process.c | 3 +-
arch/arm64/kernel/signal.c | 3 ++
6 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index c43b4ac13008..609bc44ceb8d 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -37,6 +37,8 @@ struct fpsimd_state {
u32 fpcr;
};
};
+ /* the id of the last cpu to have restored this state */
+ unsigned int last_cpu;
};
#if defined(__KERNEL__) && defined(CONFIG_COMPAT)
@@ -57,6 +59,7 @@ extern void fpsimd_load_state(struct fpsimd_state *state);
extern void fpsimd_thread_switch(struct task_struct *next);
extern void fpsimd_flush_thread(void);
+extern void fpsimd_reload_fpstate(void);
#endif
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 720e70b66ffd..4a1ca1cfb2f8 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -100,6 +100,7 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_SIGPENDING 0
#define TIF_NEED_RESCHED 1
#define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
+#define TIF_FOREIGN_FPSTATE 3 /* CPU's FP state is not current's */
#define TIF_SYSCALL_TRACE 8
#define TIF_POLLING_NRFLAG 16
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
@@ -112,10 +113,11 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
+#define _TIF_FOREIGN_FPSTATE (1 << TIF_FOREIGN_FPSTATE)
#define _TIF_32BIT (1 << TIF_32BIT)
#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
- _TIF_NOTIFY_RESUME)
+ _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE)
#endif /* __KERNEL__ */
#endif /* __ASM_THREAD_INFO_H */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 39ac630d83de..80464e2fb1a5 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -576,7 +576,7 @@ fast_work_pending:
str x0, [sp, #S_X0] // returned x0
work_pending:
tbnz x1, #TIF_NEED_RESCHED, work_resched
- /* TIF_SIGPENDING or TIF_NOTIFY_RESUME case */
+ /* TIF_SIGPENDING, TIF_NOTIFY_RESUME or TIF_FOREIGN_FPSTATE case */
ldr x2, [sp, #S_PSTATE]
mov x0, sp // 'regs'
tst x2, #PSR_MODE_MASK // user mode regs?
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 4aef42a04bdc..226a495e019c 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -35,6 +35,23 @@
#define FPEXC_IDF (1 << 7)
/*
+ * In order to reduce the number of times the fpsimd state is needlessly saved
+ * and restored, keep track here of which task's userland owns the current state
+ * of the FPSIMD register file.
+ *
+ * This percpu variable points to the fpsimd_state.last_cpu field of the task
+ * whose FPSIMD state was most recently loaded onto this cpu. The last_cpu field
+ * itself contains the id of the cpu onto which the task's FPSIMD state was
+ * loaded most recently. So, to decide whether we can skip reloading the FPSIMD
+ * state, we need to check
+ * (a) whether this task was the last one to have its FPSIMD state loaded onto
+ * this cpu
+ * (b) whether this task may have manipulated its FPSIMD state on another cpu in
+ * the meantime
+ */
+static DEFINE_PER_CPU(unsigned int *, fpsimd_last_task);
+
+/*
* Trapped FP/ASIMD access.
*/
void do_fpsimd_acc(unsigned int esr, struct pt_regs *regs)
@@ -72,18 +89,56 @@ void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
void fpsimd_thread_switch(struct task_struct *next)
{
- /* check if not kernel threads */
- if (current->mm)
+ /*
+ * The thread flag TIF_FOREIGN_FPSTATE conveys that the userland FPSIMD
+ * state belonging to the current task is not present in the registers
+ * but has (already) been saved to memory in order for the kernel to be
+ * able to go off and use the registers for something else. Therefore,
+ * we must not (re)save the register contents if this flag is set.
+ */
+ if (current->mm && !test_thread_flag(TIF_FOREIGN_FPSTATE))
fpsimd_save_state(¤t->thread.fpsimd_state);
- if (next->mm)
- fpsimd_load_state(&next->thread.fpsimd_state);
+
+ if (next->mm) {
+ /*
+ * If we are switching to a task whose most recent userland NEON
+ * contents are already in the registers of *this* cpu, we can
+ * skip loading the state from memory. Otherwise, set the
+ * TIF_FOREIGN_FPSTATE flag so the state will be loaded upon the
+ * next entry of userland.
+ */
+ struct fpsimd_state *st = &next->thread.fpsimd_state;
+
+ if (__get_cpu_var(fpsimd_last_task) == &st->last_cpu
+ && st->last_cpu == smp_processor_id())
+ clear_ti_thread_flag(task_thread_info(next),
+ TIF_FOREIGN_FPSTATE);
+ else
+ set_ti_thread_flag(task_thread_info(next),
+ TIF_FOREIGN_FPSTATE);
+ }
}
void fpsimd_flush_thread(void)
{
- preempt_disable();
memset(¤t->thread.fpsimd_state, 0, sizeof(struct fpsimd_state));
- fpsimd_load_state(¤t->thread.fpsimd_state);
+ set_thread_flag(TIF_FOREIGN_FPSTATE);
+}
+
+void fpsimd_reload_fpstate(void)
+{
+ preempt_disable();
+ if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
+ /*
+ * We are entering userland and the userland context is not yet
+ * present in the registers.
+ */
+ struct fpsimd_state *st = ¤t->thread.fpsimd_state;
+
+ fpsimd_load_state(st);
+ __get_cpu_var(fpsimd_last_task) = &st->last_cpu;
+ st->last_cpu = smp_processor_id();
+ }
preempt_enable();
}
@@ -98,16 +153,20 @@ void kernel_neon_begin(void)
BUG_ON(in_interrupt());
preempt_disable();
- if (current->mm)
+ /*
+ * Save the userland FPSIMD state if we have one and if we haven't done
+ * so already. Clear fpsimd_last_task to indicate that there is no
+ * longer userland context in the registers.
+ */
+ if (current->mm && !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
fpsimd_save_state(¤t->thread.fpsimd_state);
+ __get_cpu_var(fpsimd_last_task) = NULL;
+
}
EXPORT_SYMBOL(kernel_neon_begin);
void kernel_neon_end(void)
{
- if (current->mm)
- fpsimd_load_state(¤t->thread.fpsimd_state);
-
preempt_enable();
}
EXPORT_SYMBOL(kernel_neon_end);
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 248a15db37f2..274316df860f 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -205,7 +205,8 @@ void release_thread(struct task_struct *dead_task)
int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
{
- fpsimd_save_state(¤t->thread.fpsimd_state);
+ if (!test_thread_flag(TIF_FOREIGN_FPSTATE))
+ fpsimd_save_state(¤t->thread.fpsimd_state);
*dst = *src;
return 0;
}
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 890a591f75dd..0a9eccf4fc0f 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -416,4 +416,7 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(regs);
}
+
+ if (thread_flags & _TIF_FOREIGN_FPSTATE)
+ fpsimd_reload_fpstate();
}
--
1.8.3.2
^ permalink raw reply related
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