* [PATCH v2] dma: imx-sdma: clarify firmare not found warning
From: Lothar Waßmann @ 2014-01-21 6:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140120151152.GE15937@n2100.arm.linux.org.uk>
Hi,
Russell King - ARM Linux wrote:
> On Mon, Jan 20, 2014 at 03:09:02PM +0100, Lothar Wa?mann wrote:
> > Hi,
> >
> > Russell King - ARM Linux wrote:
> > > On Mon, Jan 20, 2014 at 12:36:27PM +0100, Lothar Wa?mann wrote:
> > > > In case user space firmware loading support (CONFIG_FW_LOADER) is
> > > > enabled, this message may still be inadequate, since the firmware may
> > > > very well be loaded lateron.
> > >
> > > I haven't worked out whether that's actually possible - I saw no way to
> > > re-trigger the firmware request, and once the firmware request expires,
> > > there seems to be no way for userspace to say "okay, the firmware is now
> > > available, please load it".
> > >
> > I can confirm that it does work, since I'm using it.
> > I have the following in my /etc/init.d/mdev.sh script:
> > | [ "$VERBOSE" = no ] || echo -n "Triggering firmware load"
> > | for dir in $(find /sys/class/firmware/ -type l);do
> > | echo add > "$dir/uevent"
> > | done
>
> I can't check on iMX6 at the moment, but I did look in that directory
> after the iMX6 SDMA reported that it hadn't found any firmware, and
> it was empty apart from the "timeout" file.
>
The file is automatically removed after the timeout has expired (or
the frimware has been loaded).
Thus you must check for it within the timeout period during boot.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply
* [patch] drm/exynos: potential use after free in exynos_drm_open()
From: Dan Carpenter @ 2014-01-21 6:57 UTC (permalink / raw)
To: linux-arm-kernel
If exynos_drm_subdrv_open() fails then we re-use "file_priv".
Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 9d096a0c5f8d..3c845292845a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
if (ret) {
kfree(file_priv);
file->driver_priv = NULL;
+ return ret;
}
anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
@@ -186,7 +187,7 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
file_priv->anon_filp = anon_filp;
- return ret;
+ return 0;
}
static void exynos_drm_preclose(struct drm_device *dev,
^ permalink raw reply related
* [patch] ARM: mvebu: checking for IS_ERR() instead of NULL
From: Dan Carpenter @ 2014-01-21 6:52 UTC (permalink / raw)
To: linux-arm-kernel
of_iomap() returns NULL on error, it doesn't return error pointers.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/arch/arm/mach-mvebu/mvebu-soc-id.c b/arch/arm/mach-mvebu/mvebu-soc-id.c
index fe4fc1cbdfaf..b4cfbd62709d 100644
--- a/arch/arm/mach-mvebu/mvebu-soc-id.c
+++ b/arch/arm/mach-mvebu/mvebu-soc-id.c
@@ -88,7 +88,7 @@ static int __init mvebu_soc_id_init(void)
}
pci_base = of_iomap(child, 0);
- if (IS_ERR(pci_base)) {
+ if (!pci_base) {
pr_err("cannot map registers\n");
ret = -ENOMEM;
goto res_ioremap;
^ permalink raw reply related
* [PATCH REPOST 5/5] ARM: kvm MMIO support BE host running LE code
From: Victor Kamensky @ 2014-01-21 6:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAhSdy28wyf4ynWwdyhMBZQ983t32n1fkM8AaNXhiew0TVAAEw@mail.gmail.com>
Hi Anup,
On 20 January 2014 21:46, Anup Patel <anup@brainfault.org> wrote:
> On Tue, Jan 21, 2014 at 10:54 AM, Victor Kamensky
> <victor.kamensky@linaro.org> wrote:
>> On 20 January 2014 17:19, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>>> On Mon, Jan 06, 2014 at 05:59:03PM -0800, Victor Kamensky wrote:
>>>> On 6 January 2014 14:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>>>> > On Mon, Jan 06, 2014 at 10:31:42PM +0000, Peter Maydell wrote:
>>>> >> On 6 January 2014 18:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> >> > No matter how data is stored in memory (BE, LE, or
>>>> >> > even PDP endianness), CPU registers always have a consistent
>>>> >> > representation. They are immune to CPU endianness change, and storing
>>>> >> > to/reading from memory won't change the value, as long as you use the
>>>> >> > same endianness for writing/reading.
>>>> >>
>>>> >> Ah, endianness. This always confuses me, but I hope the following
>>>> >> is correct... (in all the following when I say BE I mean BE8, not BE32,
>>>> >> since BE32 and virtualization never occur in the same CPU).
>>>> >>
>>>> >> Certainly registers don't have endianness, but the entire point
>>>> >> of the CPSR.E bit is exactly that it changes the value as it is
>>>> >> stored to / read from memory, isn't it? -- that's where and when the
>>>> >> byte-lane flipping happens.
>>>> >>
>>>> >> Where this impacts the hypervisor is that instead of actually sending
>>>> >> the data out to the bus via the byte-swapping h/w, we've trapped instead.
>>>> >> The hypervisor reads the original data directly from the guest CPU
>>>> >> registers, and so it's the hypervisor and userspace support code that
>>>> >> between them have to emulate the equivalent of the byte lane
>>>> >> swapping h/w. You could argue that it shouldn't be the kernel's
>>>> >> job, but since the kernel has to do it for the devices it emulates
>>>> >> internally, I'm not sure that makes much sense.
>>>> >
>>>> > As far as I understand, this is exactly what vcpu_data_guest_to_host and
>>>> > vcpu_data_host_to_guest do; emulate the byte lane swapping.
>>>> >
>>>> > The problem is that it only works on a little-endian host with the
>>>> > current code, because be16_to_cpu (for example), actually perform a
>>>> > byteswap, which is what needs to be emulated. On a big-endian host, we
>>>> > do nothing, so we end up giving a byteswapped value to the emulated
>>>> > device.
>>>>
>>>> Yes, that was my point on the thread: vcpu_data_guest_to_host and
>>>> vcpu_data_host_to_guest functions for any given host endianity should
>>>> give opposite endian results depending on CPSR E bit value. And
>>>> currently it is not happening in BE host case. It seems that Peter and
>>>> you agree with that and I gave example in another email with
>>>> dynamically switching E bit illustrating this problem for BE host.
>>>>
>>>> > I think a cleaner fix than this patch is to just change the
>>>> > be16_to_cpu() to a __swab16() instead, which clearly indicates that
>>>> > 'here is the byte lane swapping'.
>>>>
>>>> Yes, that may work, but it is a bit orthogonal issue.
>>>
>>> Why? I don't think it is, I think it's addressing exactly the point at
>>> hand.
>>>
>>>> And I don't think
>>>> it is better. For this to work one need to change canonical endianity on
>>>> one of the sides around vcpu_data_guest_to_host and
>>>> vcpu_data_host_to_guest functions.
>>>
>>> You have to simply clearly define which format you want mmio.data to be
>>> in.
>>
>> I believe it is already decided. 'mmio.data' in 'struct kvm_run' is not
>> an integer type - it is bytes array. Bytes array does not have endianity.
>> It is endian agnostic. Here is snippet from linux/kvm.h
>>
>> /* KVM_EXIT_MMIO */
>> struct {
>> __u64 phys_addr;
>> __u8 data[8];
>> __u32 len;
>> __u8 is_write;
>> } mmio;
>>
>> it is very natural to treat it as just a piece of memory. I.e when code reads
>> emulated LE device address as integer, this array will contain integer
>> placed in memory in LE order, data[3] is MSB, as it would be located in
>> regular memory. When code reads emulated BE device address as
>> integer this array will contain integer placed in memory in BE order,
>> data[0] is MSB.
>>
>> You can think about it in that way: ARM system emulator runs on x86
>> (LE) and on PPC (BE). How mmio.data array for the same emulated
>> device should look like in across these two cases? I believe it should
>> be identical - just a stream of bytes.
>>
>> Emulator code handles this situation quite nicely. For example check
>> in qemu endianness field of MemoryRegionOps structure. Depending
>> of the field value and current emulator endianity code will place
>> results into 'mmio.data' array in right order. See [1] as an example
>> in qemu where endianity of certain ARM devices were not declared
>> correctly - it was marked as DEVICE_NATIVE_ENDIAN whereas
>> it should be DEVICE_LITTLE_ENDIAN. After I changed that BE qemu
>> pretty much started working. I strongly suspect if one would run
>> ARM system emulation on PPC (BE) he/she would need the same
>> changes.
>>
>> Note issue with virtio endianity is very different problem - there it
>> is not clear for given arrangement of host/emulator how to treat
>> virtio devices as LE or BE, and in what format data in rings
>> descriptors are.
>
> IMHO, device endianess should be taken care by device emulators only
> because we can have Machine Model containing both LE devices and
> BE devices. KVM ARM/ARM64 should only worry about endianess of
> in-kernel emulated devices (e.g. VGIC). In general, QEMU or KVMTOOL
> should be responsible of device endianess and for this QEMU or KVMTOOL
> should also know whether Guest (or VM) is little-endian or big-endian.
I agree with most of above statement except last part. I think
emulator and host KVM should not really care about guest endianity.
They should work in the same way in either case. MarcZ illustrated this
earlier with setup where LE KVM hosted either LE guest or BE guest.
Also note endianity as far as emulation concerned strictly speaking is
not property of the guest, it is rather property of current CPU execution
context (i.e E bit in CPSR reg of V7) In fact access endianity can
change on the fly - i.e when BE V7 image starts initially it assumes
that it runs in LE mode, then once kernel entered it switches CPU
into BE mode, the same happens with secondary CPU callback. And
with the last one I run into situation where such callback before switching
into BE mode read some emulated device with E bit off, latter the same
kernel reads the same device register with E bit on
Thanks,
Victor
> Regards,
> Anup
>
>>
>> Thanks,
>> Victor
>>
>> [1] https://git.linaro.org/people/victor.kamensky/qemu-be.git/commitdiff/8599358f9711b7a546a2bba63b6277fbfb5b8e0c?hp=c4880f08ff9451e3d8020153e1a710ab4acee151
>>
>>> This is a user space interface across multiple architectures and
>>> therefore something you have to consider carefully and you're limited in
>>> choices to something that works with existing user space code.
>>>
>>>>
>>>> Changing it on side that faces hypervisor (code that handles guest spilled
>>>> CPU register set) does not make sense at all - if we will keep guest CPU
>>>> register set in memory in LE form and hypervisor runs in BE (BE host),
>>>> code that spills registers would need to do constant byteswaps. Also any
>>>> access by host kernel and hypervisor (all running in BE) would need to do
>>>> byteswaps while working with guest saved registers.
>>>>
>>>> Changing canonical form of data on side that faces emulator and mmio
>>>> part of kvm_run does not make sense either. kvm_run mmio.data field is
>>>> bytes array, when it comes to host kernel from emulator, it already contains
>>>> device memory in correct endian order that corresponds to endianity of
>>>> emulated device. For example for LE device word read access, after call is
>>>> emulated, mmio.data will contain mmio.data[0], mmio.data[1], mmio.data[2]
>>>> mmio.data[3] values in LE order (mmio.data[3] is MSB). Now look at
>>>> mmio_read_buf function introduced by Marc's 6d89d2d9 commit, this function
>>>> will byte copy this mmio.data buffer into integer according to ongoing mmio
>>>> access size. Note in BE host case such integer, in 'data' variable of
>>>> kvm_handle_mmio_return function, will have byteswapped value. Now when it will
>>>> be passed into vcpu_data_host_to_guest function, and it emulates read access
>>>> of guest with E bit set, and if we follow your suggestion, it will be
>>>> byteswapped.
>>>> I.e 'data' integer will contain non byteswapped value of LE device. It will be
>>>> further stored into some vcpu_reg register, still in native format (BE
>>>> store), and
>>>> further restored into guest CPU register, still non byteswapped (BE hypervisor).
>>>> And that is not what BE client reading word of LE device expects - BE client
>>>> knowing that it reads LE device with E bit set, it will issue additional rev
>>>> instruction to get device memory as integer. If we really want to follow your
>>>> suggestion, one may introduce compensatory byteswaps in mmio_read_buf
>>>> and mmio_write_buf functions in case of BE host, rather then just do
>>>> memcpy ... but I am not sure what it will buy us - in BE case it will swap data
>>>> twice.
>>>>
>>>> Note in above description by "canonical" I mean some form of data regardless
>>>> of current access CPSR E value. But it may differ depending on host endianess.
>>>>
>>>
>>> There's a lot of text to digest here, talking about a canonical form
>>> here doesn't help; just define the layout of the destination byte array.
>>> I also got completely lost in what you're referring to when you talk
>>> about 'sides' here.
>>>
>>> The thing we must decide is how the data is stored in
>>> kvm_exit_mmio.data. See Peter's recent thread "KVM and
>>> variable-endianness guest CPUs". Once we agree on this, the rest should
>>> be easy (assuming we use the same structure for the data in the kernel's
>>> internal kvm_exit_mmio declared on the stack in io_mem_abort()).
>>>
>>> The format you suggest requires any consumer of this data to consider
>>> the host endianness, which I don't think makes anything more clear (see
>>> my comment on the vgic patch).
>>>
>>> The in-kernel interface between the io_mem_abort() code and any
>>> in-kernel emulated device must do exactly the same as the interface
>>> between KVM and QEMU must do for KVM_EXIT_MMIO.
>>>
>>> --
>>> Christoffer
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm at lists.cs.columbia.edu
>> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
^ permalink raw reply
* [PATCH] pinctrl:at91:add drive strength configuration
From: Marek Roszko @ 2014-01-21 6:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121060637.GP9558@ns203013.ovh.net>
The SAMA5 and most SAM9xGs(baring a few exceptions like the SAM945G) have drive
control registers that allow configuring the output current of the gpio pins between
three predefined levels of low,medium and high drive.
This patch adds four new dt-bindings that allow setting the strength via the device tree.
Added set/get drive strength to pinctrl ops table.
A sama5d3-gpio of id entry was made to set the set/get drive strength function for the
sam5d3s which unforunately have different register addresses and strength-to-value
map than the sam9s.
Added PIO_DRIVER1/2 defines in at91_pio.h prefixed with the target SoC.
---
arch/arm/boot/dts/sama5d3.dtsi | 10 +--
arch/arm/mach-at91/include/mach/at91_pio.h | 7 ++
drivers/pinctrl/pinctrl-at91.c | 131 +++++++++++++++++++++++++++-
include/dt-bindings/pinctrl/at91.h | 5 ++
4 files changed, 147 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 5cdaba4..5576330 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -619,7 +619,7 @@
pioA: gpio at fffff200 {
- compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
+ compatible = "atmel,sama5d3-gpio", "atmel,at91rm9200-gpio";
reg = <0xfffff200 0x100>;
interrupts = <6 IRQ_TYPE_LEVEL_HIGH 1>;
#gpio-cells = <2>;
@@ -629,7 +629,7 @@
};
pioB: gpio at fffff400 {
- compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
+ compatible = "atmel,sama5d3-gpio", "atmel,at91rm9200-gpio";
reg = <0xfffff400 0x100>;
interrupts = <7 IRQ_TYPE_LEVEL_HIGH 1>;
#gpio-cells = <2>;
@@ -639,7 +639,7 @@
};
pioC: gpio at fffff600 {
- compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
+ compatible = "atmel,sama5d3-gpio", "atmel,at91rm9200-gpio";
reg = <0xfffff600 0x100>;
interrupts = <8 IRQ_TYPE_LEVEL_HIGH 1>;
#gpio-cells = <2>;
@@ -649,7 +649,7 @@
};
pioD: gpio at fffff800 {
- compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
+ compatible = "atmel,sama5d3-gpio", "atmel,at91rm9200-gpio";
reg = <0xfffff800 0x100>;
interrupts = <9 IRQ_TYPE_LEVEL_HIGH 1>;
#gpio-cells = <2>;
@@ -659,7 +659,7 @@
};
pioE: gpio at fffffa00 {
- compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
+ compatible = "atmel,sama5d3-gpio", "atmel,at91rm9200-gpio";
reg = <0xfffffa00 0x100>;
interrupts = <10 IRQ_TYPE_LEVEL_HIGH 1>;
#gpio-cells = <2>;
diff --git a/arch/arm/mach-at91/include/mach/at91_pio.h b/arch/arm/mach-at91/include/mach/at91_pio.h
index 732b11c..b1e6380 100644
--- a/arch/arm/mach-at91/include/mach/at91_pio.h
+++ b/arch/arm/mach-at91/include/mach/at91_pio.h
@@ -66,6 +66,13 @@
#define PIO_FRLHSR 0xd8 /* Fall/Rise - Low/High Status Register */
#define PIO_SCHMITT 0x100 /* Schmitt Trigger Register */
+
+#define SAMA5D3_PIO_DRIVER1 0x118 /*PIO Driver 1 register offset*/
+#define SAMA5D3_PIO_DRIVER2 0x11C /*PIO Driver 2 register offset*/
+
+#define AT91SAM9X5_PIO_DRIVER1 0x114 /*PIO Driver 1 register offset*/
+#define AT91SAM9X5_PIO_DRIVER2 0x118 /*PIO Driver 2 register offset*/
+
#define ABCDSR_PERIPH_A 0x0
#define ABCDSR_PERIPH_B 0x1
#define ABCDSR_PERIPH_C 0x2
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a7549c4..d79e085 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -62,10 +62,21 @@ static int gpio_banks;
#define DEGLITCH (1 << 2)
#define PULL_DOWN (1 << 3)
#define DIS_SCHMIT (1 << 4)
+#define DRIVE_STRENGTH_SHIFT 5
+#define DRIVE_STRENGTH (0x3 << DRIVE_STRENGTH_SHIFT)
#define DEBOUNCE (1 << 16)
#define DEBOUNCE_VAL_SHIFT 17
#define DEBOUNCE_VAL (0x3fff << DEBOUNCE_VAL_SHIFT)
+#define DRIVE_STRENGTH_DEFAULT (0x0 << 5)
+#define DRIVE_STRENGTH_LOW (0x1 << 5)
+#define DRIVE_STRENGTH_MED (0x2 << 5)
+#define DRIVE_STRENGTH_HI (0x3 << 5)
+
+
+#define DRIVE_STRENGTH_MASK 0x3
+#define NUM_PINS_PER_DIVIDED_REGS 16
+
/**
* struct at91_pmx_func - describes AT91 pinmux functions
* @name: the name of this specific function
@@ -148,6 +159,8 @@ struct at91_pinctrl_mux_ops {
void (*set_deglitch)(void __iomem *pio, unsigned mask, bool is_on);
bool (*get_debounce)(void __iomem *pio, unsigned pin, u32 *div);
void (*set_debounce)(void __iomem *pio, unsigned mask, bool is_on, u32 div);
+ unsigned (*get_drivestrength)(void __iomem *pio, unsigned pin);
+ void (*set_drivestrength)(void __iomem *pio, unsigned pin, u32 strength);
bool (*get_pulldown)(void __iomem *pio, unsigned pin);
void (*set_pulldown)(void __iomem *pio, unsigned mask, bool is_on);
bool (*get_schmitt_trig)(void __iomem *pio, unsigned pin);
@@ -319,6 +332,24 @@ static unsigned pin_to_mask(unsigned int pin)
return 1 << pin;
}
+static inline int two_bit_pin_value_shift(unsigned pin)
+{
+ return 2*((pin >= NUM_PINS_PER_DIVIDED_REGS)
+ ? pin - NUM_PINS_PER_DIVIDED_REGS : pin);
+}
+
+static unsigned sama5d3_get_drive_register(unsigned int pin)
+{
+ return (pin > NUM_PINS_PER_DIVIDED_REGS-1)
+ ? SAMA5D3_PIO_DRIVER2: SAMA5D3_PIO_DRIVER1;
+}
+
+static unsigned at91sam9x5_get_drive_register(unsigned int pin)
+{
+ return (pin > NUM_PINS_PER_DIVIDED_REGS-1)
+ ? AT91SAM9X5_PIO_DRIVER2: AT91SAM9X5_PIO_DRIVER1;
+}
+
static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
{
writel_relaxed(mask, pio + PIO_IDR);
@@ -462,6 +493,73 @@ static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is
__raw_writel(mask, pio + (is_on ? PIO_PPDER : PIO_PPDDR));
}
+static inline u32 read_drive_strength(void __iomem *reg, unsigned pin)
+{
+ unsigned tmp = __raw_readl(reg);
+ tmp = tmp >> two_bit_pin_value_shift(pin)
+
+ return tmp & DRIVE_STRENGTH_MASK;
+}
+
+static unsigned at91_mux_sama5d3_get_drivestrength(void __iomem *pio, unsigned pin)
+{
+ unsigned tmp = read_drive_strength(pio + sama5d3_get_drive_register(pin), pin);
+
+ /*value of 0 = low, all other values are 1 to 1*/
+ if (!tmp)
+ tmp = DRIVE_STRENGTH_LOW;
+
+ return tmp;
+}
+
+static unsigned at91_mux_sam9x5_get_drivestrength(void __iomem *pio, unsigned pin)
+{
+ unsigned tmp = read_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin);
+
+ /*
+ * inverse the setting for dt defines
+ * 0 = hi, 1 = med, 2 = low, 3 = rsvd
+ */
+ tmp = DRIVE_STRENGTH_HI - tmp;
+
+ return tmp;
+}
+
+static void set_drive_strength(void __iomem *reg, unsigned pin, u32 strength)
+{
+ unsigned tmp = __raw_readl(reg);
+
+ tmp &= ~(DRIVE_STRENGTH_MASK << two_bit_pin_value_shift(pin));
+ tmp |= strength << two_bit_pin_value_shift(pin);
+
+ __raw_writel(tmp, reg);
+}
+
+static void at91_mux_sama5d3_set_drivestrength(void __iomem *pio, unsigned pin,
+ u32 setting)
+{
+ /* do nothing if setting is zero */
+ if (!setting)
+ return;
+
+ /* strength is 1 to 1 with setting for SAMA5 */
+ set_drive_strength(pio + sama5d3_get_drive_register(pin), pin, setting);
+}
+
+static void at91_mux_sam9x5_set_drivestrength(void __iomem *pio, unsigned pin,
+ u32 setting)
+{
+ /* do nothing if setting is zero */
+ if (!setting)
+ return;
+
+ /* strength is inverse on SAM9s, 0 = hi, 1 = med, 2 = low, 3 = rsvd */
+ setting = DRIVE_STRENGTH_HI - setting;
+
+ set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin, setting);
+}
+
+
static void at91_mux_pio3_disable_schmitt_trig(void __iomem *pio, unsigned mask)
{
__raw_writel(__raw_readl(pio + PIO_SCHMITT) | mask, pio + PIO_SCHMITT);
@@ -491,6 +589,27 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
.set_deglitch = at91_mux_pio3_set_deglitch,
.get_debounce = at91_mux_pio3_get_debounce,
.set_debounce = at91_mux_pio3_set_debounce,
+ .get_drivestrength = at91_mux_sam9x5_get_drivestrength,
+ .set_drivestrength = at91_mux_sam9x5_set_drivestrength,
+ .get_pulldown = at91_mux_pio3_get_pulldown,
+ .set_pulldown = at91_mux_pio3_set_pulldown,
+ .get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
+ .disable_schmitt_trig = at91_mux_pio3_disable_schmitt_trig,
+ .irq_type = alt_gpio_irq_type,
+};
+
+static struct at91_pinctrl_mux_ops sama5d3_ops = {
+ .get_periph = at91_mux_pio3_get_periph,
+ .mux_A_periph = at91_mux_pio3_set_A_periph,
+ .mux_B_periph = at91_mux_pio3_set_B_periph,
+ .mux_C_periph = at91_mux_pio3_set_C_periph,
+ .mux_D_periph = at91_mux_pio3_set_D_periph,
+ .get_deglitch = at91_mux_pio3_get_deglitch,
+ .set_deglitch = at91_mux_pio3_set_deglitch,
+ .get_debounce = at91_mux_pio3_get_debounce,
+ .set_debounce = at91_mux_pio3_set_debounce,
+ .get_drivestrength = at91_mux_sama5d3_get_drivestrength,
+ .set_drivestrength = at91_mux_sama5d3_set_drivestrength,
.get_pulldown = at91_mux_pio3_get_pulldown,
.set_pulldown = at91_mux_pio3_set_pulldown,
.get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
@@ -498,6 +617,7 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
.irq_type = alt_gpio_irq_type,
};
+
static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
{
if (pin->mux) {
@@ -736,6 +856,9 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
*config |= DEGLITCH;
if (info->ops->get_debounce && info->ops->get_debounce(pio, pin, &div))
*config |= DEBOUNCE | (div << DEBOUNCE_VAL_SHIFT);
+ if (info->ops->get_drivestrength)
+ *config |= DRIVE_STRENGTH | (info->ops->get_drivestrength(pio, pin)
+ << DRIVE_STRENGTH_SHIFT);
if (info->ops->get_pulldown && info->ops->get_pulldown(pio, pin))
*config |= PULL_DOWN;
if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
@@ -753,6 +876,7 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
void __iomem *pio;
int i;
unsigned long config;
+ unsigned pin;
for (i = 0; i < num_configs; i++) {
config = configs[i];
@@ -761,7 +885,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
"%s:%d, pin_id=%d, config=0x%lx",
__func__, __LINE__, pin_id, config);
pio = pin_to_controller(info, pin_to_bank(pin_id));
- mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
+ pin = pin_id % MAX_NB_GPIO_PER_BANK;
+ mask = pin_to_mask(pin);
if (config & PULL_UP && config & PULL_DOWN)
return -EINVAL;
@@ -773,6 +898,9 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
if (info->ops->set_debounce)
info->ops->set_debounce(pio, mask, config & DEBOUNCE,
(config & DEBOUNCE_VAL) >> DEBOUNCE_VAL_SHIFT);
+ if (info->ops->set_drivestrength)
+ info->ops->set_drivestrength(pio, pin,
+ (config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
if (info->ops->set_pulldown)
info->ops->set_pulldown(pio, mask, config & PULL_DOWN);
if (info->ops->disable_schmitt_trig && config & DIS_SCHMIT)
@@ -1551,6 +1679,7 @@ static void at91_gpio_probe_fixup(void)
static struct of_device_id at91_gpio_of_match[] = {
{ .compatible = "atmel,at91sam9x5-gpio", .data = &at91sam9x5_ops, },
+ { .compatible = "atmel,sama5d3-gpio", .data = &sama5d3_ops, },
{ .compatible = "atmel,at91rm9200-gpio", .data = &at91rm9200_ops },
{ /* sentinel */ }
};
diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
index 0fee6ff..af71ebb 100644
--- a/include/dt-bindings/pinctrl/at91.h
+++ b/include/dt-bindings/pinctrl/at91.h
@@ -20,6 +20,11 @@
#define AT91_PINCTRL_PULL_UP_DEGLITCH (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DEGLITCH)
+#define AT91_PINCTRL_DRIVE_STRENGTH_DEFAULT (0x0 << 5)
+#define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x1 << 5)
+#define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x2 << 5)
+#define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x3 << 5)
+
#define AT91_PIOA 0
#define AT91_PIOB 1
#define AT91_PIOC 2
--
1.7.10.4
^ permalink raw reply related
* [PATCH REPOST 5/5] ARM: kvm MMIO support BE host running LE code
From: Christoffer Dall @ 2014-01-21 6:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAhSdy28wyf4ynWwdyhMBZQ983t32n1fkM8AaNXhiew0TVAAEw@mail.gmail.com>
On Tue, Jan 21, 2014 at 11:16:46AM +0530, Anup Patel wrote:
> On Tue, Jan 21, 2014 at 10:54 AM, Victor Kamensky
> <victor.kamensky@linaro.org> wrote:
> > On 20 January 2014 17:19, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> >> On Mon, Jan 06, 2014 at 05:59:03PM -0800, Victor Kamensky wrote:
> >>> On 6 January 2014 14:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> >>> > On Mon, Jan 06, 2014 at 10:31:42PM +0000, Peter Maydell wrote:
> >>> >> On 6 January 2014 18:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >>> >> > No matter how data is stored in memory (BE, LE, or
> >>> >> > even PDP endianness), CPU registers always have a consistent
> >>> >> > representation. They are immune to CPU endianness change, and storing
> >>> >> > to/reading from memory won't change the value, as long as you use the
> >>> >> > same endianness for writing/reading.
> >>> >>
> >>> >> Ah, endianness. This always confuses me, but I hope the following
> >>> >> is correct... (in all the following when I say BE I mean BE8, not BE32,
> >>> >> since BE32 and virtualization never occur in the same CPU).
> >>> >>
> >>> >> Certainly registers don't have endianness, but the entire point
> >>> >> of the CPSR.E bit is exactly that it changes the value as it is
> >>> >> stored to / read from memory, isn't it? -- that's where and when the
> >>> >> byte-lane flipping happens.
> >>> >>
> >>> >> Where this impacts the hypervisor is that instead of actually sending
> >>> >> the data out to the bus via the byte-swapping h/w, we've trapped instead.
> >>> >> The hypervisor reads the original data directly from the guest CPU
> >>> >> registers, and so it's the hypervisor and userspace support code that
> >>> >> between them have to emulate the equivalent of the byte lane
> >>> >> swapping h/w. You could argue that it shouldn't be the kernel's
> >>> >> job, but since the kernel has to do it for the devices it emulates
> >>> >> internally, I'm not sure that makes much sense.
> >>> >
> >>> > As far as I understand, this is exactly what vcpu_data_guest_to_host and
> >>> > vcpu_data_host_to_guest do; emulate the byte lane swapping.
> >>> >
> >>> > The problem is that it only works on a little-endian host with the
> >>> > current code, because be16_to_cpu (for example), actually perform a
> >>> > byteswap, which is what needs to be emulated. On a big-endian host, we
> >>> > do nothing, so we end up giving a byteswapped value to the emulated
> >>> > device.
> >>>
> >>> Yes, that was my point on the thread: vcpu_data_guest_to_host and
> >>> vcpu_data_host_to_guest functions for any given host endianity should
> >>> give opposite endian results depending on CPSR E bit value. And
> >>> currently it is not happening in BE host case. It seems that Peter and
> >>> you agree with that and I gave example in another email with
> >>> dynamically switching E bit illustrating this problem for BE host.
> >>>
> >>> > I think a cleaner fix than this patch is to just change the
> >>> > be16_to_cpu() to a __swab16() instead, which clearly indicates that
> >>> > 'here is the byte lane swapping'.
> >>>
> >>> Yes, that may work, but it is a bit orthogonal issue.
> >>
> >> Why? I don't think it is, I think it's addressing exactly the point at
> >> hand.
> >>
> >>> And I don't think
> >>> it is better. For this to work one need to change canonical endianity on
> >>> one of the sides around vcpu_data_guest_to_host and
> >>> vcpu_data_host_to_guest functions.
> >>
> >> You have to simply clearly define which format you want mmio.data to be
> >> in.
> >
> > I believe it is already decided. 'mmio.data' in 'struct kvm_run' is not
> > an integer type - it is bytes array. Bytes array does not have endianity.
> > It is endian agnostic. Here is snippet from linux/kvm.h
> >
> > /* KVM_EXIT_MMIO */
> > struct {
> > __u64 phys_addr;
> > __u8 data[8];
> > __u32 len;
> > __u8 is_write;
> > } mmio;
> >
> > it is very natural to treat it as just a piece of memory. I.e when code reads
> > emulated LE device address as integer, this array will contain integer
> > placed in memory in LE order, data[3] is MSB, as it would be located in
> > regular memory. When code reads emulated BE device address as
> > integer this array will contain integer placed in memory in BE order,
> > data[0] is MSB.
> >
> > You can think about it in that way: ARM system emulator runs on x86
> > (LE) and on PPC (BE). How mmio.data array for the same emulated
> > device should look like in across these two cases? I believe it should
> > be identical - just a stream of bytes.
> >
> > Emulator code handles this situation quite nicely. For example check
> > in qemu endianness field of MemoryRegionOps structure. Depending
> > of the field value and current emulator endianity code will place
> > results into 'mmio.data' array in right order. See [1] as an example
> > in qemu where endianity of certain ARM devices were not declared
> > correctly - it was marked as DEVICE_NATIVE_ENDIAN whereas
> > it should be DEVICE_LITTLE_ENDIAN. After I changed that BE qemu
> > pretty much started working. I strongly suspect if one would run
> > ARM system emulation on PPC (BE) he/she would need the same
> > changes.
> >
> > Note issue with virtio endianity is very different problem - there it
> > is not clear for given arrangement of host/emulator how to treat
> > virtio devices as LE or BE, and in what format data in rings
> > descriptors are.
>
> IMHO, device endianess should be taken care by device emulators only
> because we can have Machine Model containing both LE devices and
> BE devices. KVM ARM/ARM64 should only worry about endianess of
> in-kernel emulated devices (e.g. VGIC). In general, QEMU or KVMTOOL
> should be responsible of device endianess and for this QEMU or KVMTOOL
> should also know whether Guest (or VM) is little-endian or big-endian.
>
Specifying the interface to say that this is a store of the register
value directly using the endianness of the host kernel is an option.
However, user space must fetch the CPSR on each MMIO from the kernel and
look at the E-bit to understand how it should interpret the data, which
may add overhead, and it doesn't change the fact that this needs to be
specified in the API.
The E bit on ARM specifies that the CPU will swap the bytes before
putting the register value on the memory bus. That's all it does.
Something has to emulate this, and given that KVM emulates the CPU, I
think KVM should emulate the E-bit.
>From my point of view, the mmio.data API as the signal you would receive
if you're any consumer of the memory operation externally to the CPU,
which would be in the form of a bunch of wires and a length, with no
endianness.
But, the thread I pointed Victor to is focused purely on this
discussion, so you should probably respond there.
-Christoffer
>
> >
> > Thanks,
> > Victor
> >
> > [1] https://git.linaro.org/people/victor.kamensky/qemu-be.git/commitdiff/8599358f9711b7a546a2bba63b6277fbfb5b8e0c?hp=c4880f08ff9451e3d8020153e1a710ab4acee151
> >
> >> This is a user space interface across multiple architectures and
> >> therefore something you have to consider carefully and you're limited in
> >> choices to something that works with existing user space code.
> >>
> >>>
> >>> Changing it on side that faces hypervisor (code that handles guest spilled
> >>> CPU register set) does not make sense at all - if we will keep guest CPU
> >>> register set in memory in LE form and hypervisor runs in BE (BE host),
> >>> code that spills registers would need to do constant byteswaps. Also any
> >>> access by host kernel and hypervisor (all running in BE) would need to do
> >>> byteswaps while working with guest saved registers.
> >>>
> >>> Changing canonical form of data on side that faces emulator and mmio
> >>> part of kvm_run does not make sense either. kvm_run mmio.data field is
> >>> bytes array, when it comes to host kernel from emulator, it already contains
> >>> device memory in correct endian order that corresponds to endianity of
> >>> emulated device. For example for LE device word read access, after call is
> >>> emulated, mmio.data will contain mmio.data[0], mmio.data[1], mmio.data[2]
> >>> mmio.data[3] values in LE order (mmio.data[3] is MSB). Now look at
> >>> mmio_read_buf function introduced by Marc's 6d89d2d9 commit, this function
> >>> will byte copy this mmio.data buffer into integer according to ongoing mmio
> >>> access size. Note in BE host case such integer, in 'data' variable of
> >>> kvm_handle_mmio_return function, will have byteswapped value. Now when it will
> >>> be passed into vcpu_data_host_to_guest function, and it emulates read access
> >>> of guest with E bit set, and if we follow your suggestion, it will be
> >>> byteswapped.
> >>> I.e 'data' integer will contain non byteswapped value of LE device. It will be
> >>> further stored into some vcpu_reg register, still in native format (BE
> >>> store), and
> >>> further restored into guest CPU register, still non byteswapped (BE hypervisor).
> >>> And that is not what BE client reading word of LE device expects - BE client
> >>> knowing that it reads LE device with E bit set, it will issue additional rev
> >>> instruction to get device memory as integer. If we really want to follow your
> >>> suggestion, one may introduce compensatory byteswaps in mmio_read_buf
> >>> and mmio_write_buf functions in case of BE host, rather then just do
> >>> memcpy ... but I am not sure what it will buy us - in BE case it will swap data
> >>> twice.
> >>>
> >>> Note in above description by "canonical" I mean some form of data regardless
> >>> of current access CPSR E value. But it may differ depending on host endianess.
> >>>
> >>
> >> There's a lot of text to digest here, talking about a canonical form
> >> here doesn't help; just define the layout of the destination byte array.
> >> I also got completely lost in what you're referring to when you talk
> >> about 'sides' here.
> >>
> >> The thing we must decide is how the data is stored in
> >> kvm_exit_mmio.data. See Peter's recent thread "KVM and
> >> variable-endianness guest CPUs". Once we agree on this, the rest should
> >> be easy (assuming we use the same structure for the data in the kernel's
> >> internal kvm_exit_mmio declared on the stack in io_mem_abort()).
> >>
> >> The format you suggest requires any consumer of this data to consider
> >> the host endianness, which I don't think makes anything more clear (see
> >> my comment on the vgic patch).
> >>
> >> The in-kernel interface between the io_mem_abort() code and any
> >> in-kernel emulated device must do exactly the same as the interface
> >> between KVM and QEMU must do for KVM_EXIT_MMIO.
> >>
> >> --
> >> Christoffer
> > _______________________________________________
> > kvmarm mailing list
> > kvmarm at lists.cs.columbia.edu
> > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
--
Christoffer
^ permalink raw reply
* pinctrl: at91: drive strength control
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-21 6:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DE0B1E.90106@gmail.com>
please put a better commemt
On 00:52 Tue 21 Jan , Marek Roszko wrote:
> Alright, added a SAMA5D3 compatible type and gave it's own drive
> strength ops. Moved the defines to inline funcs. Made the bindings
> independent of soc.
>
> Hopefully tabs appear this time, I haven't figured out the whole mailing
> list - email client thing and finding a client that doesn't convert
> spaces/tabs is hard.
those comments
>
> ---
come here fater the ---
please use git send-email or we can not even apply the patch
> arch/arm/mach-at91/include/mach/at91sam9x5.h | 3 +
> arch/arm/mach-at91/include/mach/sama5d3.h | 3 +
> drivers/pinctrl/pinctrl-at91.c | 123
> +++++++++++++++++++++++++-
> include/dt-bindings/pinctrl/at91.h | 5 ++
> 4 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/include/mach/at91sam9x5.h
> b/arch/arm/mach-at91/include/mach/at91sam9x5.h
> index 2fc76c4..2efeb0a 100644
> --- a/arch/arm/mach-at91/include/mach/at91sam9x5.h
> +++ b/arch/arm/mach-at91/include/mach/at91sam9x5.h
> @@ -59,6 +59,9 @@
> */
> #define AT91SAM9X5_BASE_RTC 0xfffffeb0
>
> +#define AT91SAM9X5_PIO_DRIVER1 0x114 /*PIO Driver 1 register offset*/
> +#define AT91SAM9X5_PIO_DRIVER2 0x118 /*PIO Driver 2 register offset*/
move this in at91_pio.h
> +
> /*
> * Internal Memory.
> */
> diff --git a/arch/arm/mach-at91/include/mach/sama5d3.h
> b/arch/arm/mach-at91/include/mach/sama5d3.h
> index 25613d8..5d434f9 100644
> --- a/arch/arm/mach-at91/include/mach/sama5d3.h
> +++ b/arch/arm/mach-at91/include/mach/sama5d3.h
> @@ -77,6 +77,9 @@
> */
> #define SAMA5D3_BASE_RTC 0xfffffeb0
>
> +#define SAMA5D3_PIO_DRIVER1 0x118 /*PIO Driver 1 register offset*/
> +#define SAMA5D3_PIO_DRIVER2 0x11C /*PIO Driver 2 register offset*/
> +
ditto
> /*
> * Internal Memory
> */
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a7549c4..984c3be 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -62,10 +62,21 @@ static int gpio_banks;
> #define DEGLITCH (1 << 2)
> #define PULL_DOWN (1 << 3)
> #define DIS_SCHMIT (1 << 4)
> +#define DRIVE_STRENGTH_SHIFT 5
> +#define DRIVE_STRENGTH (0x3 << DRIVE_STRENGTH_SHIFT)
> #define DEBOUNCE (1 << 16)
> #define DEBOUNCE_VAL_SHIFT 17
> #define DEBOUNCE_VAL (0x3fff << DEBOUNCE_VAL_SHIFT)
>
> +#define DRIVE_STRENGTH_DEFAULT (0x0 << 5)
> +#define DRIVE_STRENGTH_LOW (0x1 << 5)
> +#define DRIVE_STRENGTH_MED (0x2 << 5)
> +#define DRIVE_STRENGTH_HI (0x3 << 5)
> +
> +
> +#define DRIVE_STRENGTH_MASK 0x3
> +#define NUM_PINS_PER_DIVIDED_REGS 16
> +
> /**
> * struct at91_pmx_func - describes AT91 pinmux functions
> * @name: the name of this specific function
> @@ -148,6 +159,8 @@ struct at91_pinctrl_mux_ops {
> void (*set_deglitch)(void __iomem *pio, unsigned mask, bool is_on);
> bool (*get_debounce)(void __iomem *pio, unsigned pin, u32 *div);
> void (*set_debounce)(void __iomem *pio, unsigned mask, bool is_on, u32
> div);
> + unsigned (*get_drivestrength)(void __iomem *pio, unsigned pin);
> + void (*set_drivestrength)(void __iomem *pio, unsigned pin, u32 strength);
> bool (*get_pulldown)(void __iomem *pio, unsigned pin);
> void (*set_pulldown)(void __iomem *pio, unsigned mask, bool is_on);
> bool (*get_schmitt_trig)(void __iomem *pio, unsigned pin);
> @@ -319,6 +332,22 @@ static unsigned pin_to_mask(unsigned int pin)
> return 1 << pin;
> }?
??
>
> +static inline int two_bit_pin_value_shift(unsigned pin)
> +{
> + return 2*((pin >= NUM_PINS_PER_DIVIDED_REGS) ? pin -
2 * (
> NUM_PINS_PER_DIVIDED_REGS : pin);
> +}
> +
> +
one line
> +static unsigned sama5d3_get_drive_register(unsigned int pin)
> +{
> + return (pin > NUM_PINS_PER_DIVIDED_REGS-1) ? SAMA5D3_PIO_DRIVER2 :
S - 1
> SAMA5D3_PIO_DRIVER1;
> +}
> +
> +static unsigned at91sam9x5_get_drive_register(unsigned int pin)
> +{
> + return (pin > NUM_PINS_PER_DIVIDED_REGS-1) ? AT91SAM9X5_PIO_DRIVER2 :
> AT91SAM9X5_PIO_DRIVER1;
> +}
> +
> static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
> {
> writel_relaxed(mask, pio + PIO_IDR);
> @@ -462,6 +491,67 @@ static void at91_mux_pio3_set_pulldown(void __iomem
> *pio, unsigned mask, bool is
> __raw_writel(mask, pio + (is_on ? PIO_PPDER : PIO_PPDDR));
> }?
>
???
> +static inline u32 read_drive_strength(void __iomem *reg, unsigned pin)
> +{
> + return (__raw_readl(reg)
> + >> two_bit_pin_value_shift(pin)) & DRIVE_STRENGTH_MASK;
use a temp val to make the code readable
> +}
> +
> +static unsigned at91_mux_sama5d3_get_drivestrength(void __iomem *pio,
> unsigned pin)
> +{
> + unsigned tmp = read_drive_strength(pio +
> sama5d3_get_drive_register(pin), pin);
> +
> + if (!tmp)
> + tmp = DRIVE_STRENGTH_LOW;
> +
> + return tmp;
> +}
> +
> +static unsigned at91_mux_sam9x5_get_drivestrength(void __iomem *pio,
> unsigned pin)
> +{
> + unsigned tmp = read_drive_strength(pio +
> at91sam9x5_get_drive_register(pin), pin);
> +
> + /*inverse settings*/
> + tmp = DRIVE_STRENGTH_HI - tmp;
> +
> + return tmp;
> +}
> +
> +static void set_drive_strength(void __iomem *reg, unsigned pin, u32
> strength)
> +{
> + unsigned tmp = __raw_readl(reg);
> +
> + tmp &= ~(DRIVE_STRENGTH_MASK << two_bit_pin_value_shift(pin));
> + tmp |= strength << two_bit_pin_value_shift(pin);
> +
> + __raw_writel(tmp, reg);
> +}
> +
> +static void at91_mux_sama5d3_set_drivestrength(void __iomem *pio,
> unsigned pin,
> + u32 setting)
> +{
> + /* do nothing if setting is zero */
> + if (!setting)
> + return;
> +
> + /* strength is 1 to 1 with setting for SAMA5 */
> + set_drive_strength(pio + sama5d3_get_drive_register(pin), pin, setting);
> +}
> +
> +static void at91_mux_sam9x5_set_drivestrength(void __iomem *pio,
> unsigned pin,
> + u32 setting)
> +{
> + /* do nothing if setting is zero */
> + if (!setting)
> + return;
> +
> + /* strength is inverse on SAM9s, 0 = hi, 1 = med, 2 = low, 3 = rsvd */
> + setting = DRIVE_STRENGTH_HI - setting;
> +
> + set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin,
> setting);
> +}
> +
> +
> static void at91_mux_pio3_disable_schmitt_trig(void __iomem *pio,
> unsigned mask)
> {
> __raw_writel(__raw_readl(pio + PIO_SCHMITT) | mask, pio + PIO_SCHMITT);
> @@ -491,6 +581,8 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
> .set_deglitch = at91_mux_pio3_set_deglitch,
> .get_debounce = at91_mux_pio3_get_debounce,
> .set_debounce = at91_mux_pio3_set_debounce,
> + .get_drivestrength = at91_mux_sam9x5_get_drivestrength,
> + .set_drivestrength = at91_mux_sam9x5_set_drivestrength,
> .get_pulldown = at91_mux_pio3_get_pulldown,
> .set_pulldown = at91_mux_pio3_set_pulldown,
> .get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
> @@ -498,6 +590,26 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
> .irq_type = alt_gpio_irq_type,
> };
>
> +static struct at91_pinctrl_mux_ops sama5d3_ops = {
> + .get_periph = at91_mux_pio3_get_periph,
> + .mux_A_periph = at91_mux_pio3_set_A_periph,
> + .mux_B_periph = at91_mux_pio3_set_B_periph,
> + .mux_C_periph = at91_mux_pio3_set_C_periph,
> + .mux_D_periph = at91_mux_pio3_set_D_periph,
> + .get_deglitch = at91_mux_pio3_get_deglitch,
> + .set_deglitch = at91_mux_pio3_set_deglitch,
> + .get_debounce = at91_mux_pio3_get_debounce,
> + .set_debounce = at91_mux_pio3_set_debounce,
> + .get_drivestrength = at91_mux_sama5d3_get_drivestrength,
> + .set_drivestrength = at91_mux_sama5d3_set_drivestrength,
> + .get_pulldown = at91_mux_pio3_get_pulldown,
> + .set_pulldown = at91_mux_pio3_set_pulldown,
> + .get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
> + .disable_schmitt_trig = at91_mux_pio3_disable_schmitt_trig,
> + .irq_type = alt_gpio_irq_type,
> +};
> +
> +
> static void at91_pin_dbg(const struct device *dev, const struct
> at91_pmx_pin *pin)
> {
> if (pin->mux) {
> @@ -736,6 +848,9 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config |= DEGLITCH;
> if (info->ops->get_debounce && info->ops->get_debounce(pio, pin, &div))
> *config |= DEBOUNCE | (div << DEBOUNCE_VAL_SHIFT);
> + if (info->ops->get_drivestrength)
> + *config |= DRIVE_STRENGTH | (info->ops->get_drivestrength(pio, pin)
> + << DRIVE_STRENGTH_SHIFT);
> if (info->ops->get_pulldown && info->ops->get_pulldown(pio, pin))
> *config |= PULL_DOWN;
> if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
> @@ -753,6 +868,7 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> void __iomem *pio;
> int i;
> unsigned long config;
> + unsigned pin;
>
> for (i = 0; i < num_configs; i++) {
> config = configs[i];
> @@ -761,7 +877,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> - mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
> + pin = pin_id % MAX_NB_GPIO_PER_BANK;
> + mask = pin_to_mask(pin);
>
> if (config & PULL_UP && config & PULL_DOWN)
> return -EINVAL;
> @@ -773,6 +890,9 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> if (info->ops->set_debounce)
> info->ops->set_debounce(pio, mask, config & DEBOUNCE,
> (config & DEBOUNCE_VAL) >> DEBOUNCE_VAL_SHIFT);
> + if (info->ops->set_drivestrength)
> + info->ops->set_drivestrength(pio, pin,
> + (config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
> if (info->ops->set_pulldown)
> info->ops->set_pulldown(pio, mask, config & PULL_DOWN);
> if (info->ops->disable_schmitt_trig && config & DIS_SCHMIT)
> @@ -1551,6 +1671,7 @@ static void at91_gpio_probe_fixup(void)
>
> static struct of_device_id at91_gpio_of_match[] = {
> { .compatible = "atmel,at91sam9x5-gpio", .data = &at91sam9x5_ops, },
> + { .compatible = "atmel,sama5d3-gpio", .data = &sama5d3_ops, },
you must update the binding too
and the dts
> { .compatible = "atmel,at91rm9200-gpio", .data = &at91rm9200_ops },
> { /* sentinel */ }
> };
> diff --git a/include/dt-bindings/pinctrl/at91.h
> b/include/dt-bindings/pinctrl/at91.h
> index 0fee6ff..af71ebb 100644
> --- a/include/dt-bindings/pinctrl/at91.h
> +++ b/include/dt-bindings/pinctrl/at91.h
> @@ -20,6 +20,11 @@
>
> #define AT91_PINCTRL_PULL_UP_DEGLITCH (AT91_PINCTRL_PULL_UP |
> AT91_PINCTRL_DEGLITCH)
>
> +#define AT91_PINCTRL_DRIVE_STRENGTH_DEFAULT (0x0 << 5)
> +#define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x1 << 5)
> +#define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x2 << 5)
> +#define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x3 << 5)
> +
> #define AT91_PIOA 0
> #define AT91_PIOB 1
> #define AT91_PIOC 2
> --
> 1.7.10.4
>
^ permalink raw reply
* [PATCH REPOST 5/5] ARM: kvm MMIO support BE host running LE code
From: Christoffer Dall @ 2014-01-21 6:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAA3XUr23_Ea8feJNiqV_6ferDCiKVDiE8nvHKqS88tjBeckc9A@mail.gmail.com>
On Mon, Jan 20, 2014 at 09:24:10PM -0800, Victor Kamensky wrote:
> On 20 January 2014 17:19, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > On Mon, Jan 06, 2014 at 05:59:03PM -0800, Victor Kamensky wrote:
> >> On 6 January 2014 14:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> >> > On Mon, Jan 06, 2014 at 10:31:42PM +0000, Peter Maydell wrote:
> >> >> On 6 January 2014 18:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >> >> > No matter how data is stored in memory (BE, LE, or
> >> >> > even PDP endianness), CPU registers always have a consistent
> >> >> > representation. They are immune to CPU endianness change, and storing
> >> >> > to/reading from memory won't change the value, as long as you use the
> >> >> > same endianness for writing/reading.
> >> >>
> >> >> Ah, endianness. This always confuses me, but I hope the following
> >> >> is correct... (in all the following when I say BE I mean BE8, not BE32,
> >> >> since BE32 and virtualization never occur in the same CPU).
> >> >>
> >> >> Certainly registers don't have endianness, but the entire point
> >> >> of the CPSR.E bit is exactly that it changes the value as it is
> >> >> stored to / read from memory, isn't it? -- that's where and when the
> >> >> byte-lane flipping happens.
> >> >>
> >> >> Where this impacts the hypervisor is that instead of actually sending
> >> >> the data out to the bus via the byte-swapping h/w, we've trapped instead.
> >> >> The hypervisor reads the original data directly from the guest CPU
> >> >> registers, and so it's the hypervisor and userspace support code that
> >> >> between them have to emulate the equivalent of the byte lane
> >> >> swapping h/w. You could argue that it shouldn't be the kernel's
> >> >> job, but since the kernel has to do it for the devices it emulates
> >> >> internally, I'm not sure that makes much sense.
> >> >
> >> > As far as I understand, this is exactly what vcpu_data_guest_to_host and
> >> > vcpu_data_host_to_guest do; emulate the byte lane swapping.
> >> >
> >> > The problem is that it only works on a little-endian host with the
> >> > current code, because be16_to_cpu (for example), actually perform a
> >> > byteswap, which is what needs to be emulated. On a big-endian host, we
> >> > do nothing, so we end up giving a byteswapped value to the emulated
> >> > device.
> >>
> >> Yes, that was my point on the thread: vcpu_data_guest_to_host and
> >> vcpu_data_host_to_guest functions for any given host endianity should
> >> give opposite endian results depending on CPSR E bit value. And
> >> currently it is not happening in BE host case. It seems that Peter and
> >> you agree with that and I gave example in another email with
> >> dynamically switching E bit illustrating this problem for BE host.
> >>
> >> > I think a cleaner fix than this patch is to just change the
> >> > be16_to_cpu() to a __swab16() instead, which clearly indicates that
> >> > 'here is the byte lane swapping'.
> >>
> >> Yes, that may work, but it is a bit orthogonal issue.
> >
> > Why? I don't think it is, I think it's addressing exactly the point at
> > hand.
> >
> >> And I don't think
> >> it is better. For this to work one need to change canonical endianity on
> >> one of the sides around vcpu_data_guest_to_host and
> >> vcpu_data_host_to_guest functions.
> >
> > You have to simply clearly define which format you want mmio.data to be
> > in.
>
> I believe it is already decided. 'mmio.data' in 'struct kvm_run' is not
> an integer type - it is bytes array. Bytes array does not have endianity.
Please read through this thread:
https://lists.cs.columbia.edu/pipermail/kvmarm/2014-January/008784.html
> It is endian agnostic. Here is snippet from linux/kvm.h
>
> /* KVM_EXIT_MMIO */
> struct {
> __u64 phys_addr;
> __u8 data[8];
> __u32 len;
> __u8 is_write;
> } mmio;
Thanks, I already knew where to find this though ;)
I realize that it is a byte array. But that doesn't change the fact
that a store of a word would have to put either the most or least
significant byte in data[0].
>
> it is very natural to treat it as just a piece of memory. I.e when code reads
> emulated LE device address as integer, this array will contain integer
> placed in memory in LE order, data[3] is MSB, as it would be located in
> regular memory. When code reads emulated BE device address as
> integer this array will contain integer placed in memory in BE order,
> data[0] is MSB.
I don't understand this. "code reads emulated device address as
integer". The format of the byte array cannot be device-specific,
because the kernel doesn't know about device. It can only depend on the
endianness of the VM and of the host.
Can you try in a single sentence to to specify what the format of the
byte array is?
>
> You can think about it in that way: ARM system emulator runs on x86
> (LE) and on PPC (BE). How mmio.data array for the same emulated
> device should look like in across these two cases? I believe it should
> be identical - just a stream of bytes.
Well, KVM/ARM cannot run on PPC for obvious reasons, and this is a KVM
kernel to user space interface.
>
> Emulator code handles this situation quite nicely. For example check
> in qemu endianness field of MemoryRegionOps structure. Depending
> of the field value and current emulator endianity code will place
> results into 'mmio.data' array in right order. See [1] as an example
> in qemu where endianity of certain ARM devices were not declared
> correctly - it was marked as DEVICE_NATIVE_ENDIAN whereas
> it should be DEVICE_LITTLE_ENDIAN. After I changed that BE qemu
> pretty much started working. I strongly suspect if one would run
> ARM system emulation on PPC (BE) he/she would need the same
> changes.
It doesn't really matter what the emulator does if there's no clear
specification of the interface it relies on. It may happen to work in
the cases that are already supported (by chance), but we don't know how
to deal with a new (cross-endianness situation) because it is not
specified.
>
> Note issue with virtio endianity is very different problem - there it
> is not clear for given arrangement of host/emulator how to treat
> virtio devices as LE or BE, and in what format data in rings
> descriptors are.
>
> Thanks,
> Victor
>
> [1] https://git.linaro.org/people/victor.kamensky/qemu-be.git/commitdiff/8599358f9711b7a546a2bba63b6277fbfb5b8e0c?hp=c4880f08ff9451e3d8020153e1a710ab4acee151
>
> > This is a user space interface across multiple architectures and
> > therefore something you have to consider carefully and you're limited in
> > choices to something that works with existing user space code.
> >
> >>
> >> Changing it on side that faces hypervisor (code that handles guest spilled
> >> CPU register set) does not make sense at all - if we will keep guest CPU
> >> register set in memory in LE form and hypervisor runs in BE (BE host),
> >> code that spills registers would need to do constant byteswaps. Also any
> >> access by host kernel and hypervisor (all running in BE) would need to do
> >> byteswaps while working with guest saved registers.
> >>
> >> Changing canonical form of data on side that faces emulator and mmio
> >> part of kvm_run does not make sense either. kvm_run mmio.data field is
> >> bytes array, when it comes to host kernel from emulator, it already contains
> >> device memory in correct endian order that corresponds to endianity of
> >> emulated device. For example for LE device word read access, after call is
> >> emulated, mmio.data will contain mmio.data[0], mmio.data[1], mmio.data[2]
> >> mmio.data[3] values in LE order (mmio.data[3] is MSB). Now look at
> >> mmio_read_buf function introduced by Marc's 6d89d2d9 commit, this function
> >> will byte copy this mmio.data buffer into integer according to ongoing mmio
> >> access size. Note in BE host case such integer, in 'data' variable of
> >> kvm_handle_mmio_return function, will have byteswapped value. Now when it will
> >> be passed into vcpu_data_host_to_guest function, and it emulates read access
> >> of guest with E bit set, and if we follow your suggestion, it will be
> >> byteswapped.
> >> I.e 'data' integer will contain non byteswapped value of LE device. It will be
> >> further stored into some vcpu_reg register, still in native format (BE
> >> store), and
> >> further restored into guest CPU register, still non byteswapped (BE hypervisor).
> >> And that is not what BE client reading word of LE device expects - BE client
> >> knowing that it reads LE device with E bit set, it will issue additional rev
> >> instruction to get device memory as integer. If we really want to follow your
> >> suggestion, one may introduce compensatory byteswaps in mmio_read_buf
> >> and mmio_write_buf functions in case of BE host, rather then just do
> >> memcpy ... but I am not sure what it will buy us - in BE case it will swap data
> >> twice.
> >>
> >> Note in above description by "canonical" I mean some form of data regardless
> >> of current access CPSR E value. But it may differ depending on host endianess.
> >>
> >
> > There's a lot of text to digest here, talking about a canonical form
> > here doesn't help; just define the layout of the destination byte array.
> > I also got completely lost in what you're referring to when you talk
> > about 'sides' here.
> >
> > The thing we must decide is how the data is stored in
> > kvm_exit_mmio.data. See Peter's recent thread "KVM and
> > variable-endianness guest CPUs". Once we agree on this, the rest should
> > be easy (assuming we use the same structure for the data in the kernel's
> > internal kvm_exit_mmio declared on the stack in io_mem_abort()).
> >
> > The format you suggest requires any consumer of this data to consider
> > the host endianness, which I don't think makes anything more clear (see
> > my comment on the vgic patch).
> >
> > The in-kernel interface between the io_mem_abort() code and any
> > in-kernel emulated device must do exactly the same as the interface
> > between KVM and QEMU must do for KVM_EXIT_MMIO.
> >
> > --
> > Christoffer
--
Christoffer
^ permalink raw reply
* pinctrl: at91: drive strength control
From: Marek Roszko @ 2014-01-21 5:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121040922.GN9558@ns203013.ovh.net>
Alright, added a SAMA5D3 compatible type and gave it's own drive
strength ops. Moved the defines to inline funcs. Made the bindings
independent of soc.
Hopefully tabs appear this time, I haven't figured out the whole mailing
list - email client thing and finding a client that doesn't convert
spaces/tabs is hard.
---
arch/arm/mach-at91/include/mach/at91sam9x5.h | 3 +
arch/arm/mach-at91/include/mach/sama5d3.h | 3 +
drivers/pinctrl/pinctrl-at91.c | 123
+++++++++++++++++++++++++-
include/dt-bindings/pinctrl/at91.h | 5 ++
4 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-at91/include/mach/at91sam9x5.h
b/arch/arm/mach-at91/include/mach/at91sam9x5.h
index 2fc76c4..2efeb0a 100644
--- a/arch/arm/mach-at91/include/mach/at91sam9x5.h
+++ b/arch/arm/mach-at91/include/mach/at91sam9x5.h
@@ -59,6 +59,9 @@
*/
#define AT91SAM9X5_BASE_RTC 0xfffffeb0
+#define AT91SAM9X5_PIO_DRIVER1 0x114 /*PIO Driver 1 register offset*/
+#define AT91SAM9X5_PIO_DRIVER2 0x118 /*PIO Driver 2 register offset*/
+
/*
* Internal Memory.
*/
diff --git a/arch/arm/mach-at91/include/mach/sama5d3.h
b/arch/arm/mach-at91/include/mach/sama5d3.h
index 25613d8..5d434f9 100644
--- a/arch/arm/mach-at91/include/mach/sama5d3.h
+++ b/arch/arm/mach-at91/include/mach/sama5d3.h
@@ -77,6 +77,9 @@
*/
#define SAMA5D3_BASE_RTC 0xfffffeb0
+#define SAMA5D3_PIO_DRIVER1 0x118 /*PIO Driver 1 register offset*/
+#define SAMA5D3_PIO_DRIVER2 0x11C /*PIO Driver 2 register offset*/
+
/*
* Internal Memory
*/
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a7549c4..984c3be 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -62,10 +62,21 @@ static int gpio_banks;
#define DEGLITCH (1 << 2)
#define PULL_DOWN (1 << 3)
#define DIS_SCHMIT (1 << 4)
+#define DRIVE_STRENGTH_SHIFT 5
+#define DRIVE_STRENGTH (0x3 << DRIVE_STRENGTH_SHIFT)
#define DEBOUNCE (1 << 16)
#define DEBOUNCE_VAL_SHIFT 17
#define DEBOUNCE_VAL (0x3fff << DEBOUNCE_VAL_SHIFT)
+#define DRIVE_STRENGTH_DEFAULT (0x0 << 5)
+#define DRIVE_STRENGTH_LOW (0x1 << 5)
+#define DRIVE_STRENGTH_MED (0x2 << 5)
+#define DRIVE_STRENGTH_HI (0x3 << 5)
+
+
+#define DRIVE_STRENGTH_MASK 0x3
+#define NUM_PINS_PER_DIVIDED_REGS 16
+
/**
* struct at91_pmx_func - describes AT91 pinmux functions
* @name: the name of this specific function
@@ -148,6 +159,8 @@ struct at91_pinctrl_mux_ops {
void (*set_deglitch)(void __iomem *pio, unsigned mask, bool is_on);
bool (*get_debounce)(void __iomem *pio, unsigned pin, u32 *div);
void (*set_debounce)(void __iomem *pio, unsigned mask, bool is_on, u32
div);
+ unsigned (*get_drivestrength)(void __iomem *pio, unsigned pin);
+ void (*set_drivestrength)(void __iomem *pio, unsigned pin, u32 strength);
bool (*get_pulldown)(void __iomem *pio, unsigned pin);
void (*set_pulldown)(void __iomem *pio, unsigned mask, bool is_on);
bool (*get_schmitt_trig)(void __iomem *pio, unsigned pin);
@@ -319,6 +332,22 @@ static unsigned pin_to_mask(unsigned int pin)
return 1 << pin;
}?
+static inline int two_bit_pin_value_shift(unsigned pin)
+{
+ return 2*((pin >= NUM_PINS_PER_DIVIDED_REGS) ? pin -
NUM_PINS_PER_DIVIDED_REGS : pin);
+}
+
+
+static unsigned sama5d3_get_drive_register(unsigned int pin)
+{
+ return (pin > NUM_PINS_PER_DIVIDED_REGS-1) ? SAMA5D3_PIO_DRIVER2 :
SAMA5D3_PIO_DRIVER1;
+}
+
+static unsigned at91sam9x5_get_drive_register(unsigned int pin)
+{
+ return (pin > NUM_PINS_PER_DIVIDED_REGS-1) ? AT91SAM9X5_PIO_DRIVER2 :
AT91SAM9X5_PIO_DRIVER1;
+}
+
static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
{
writel_relaxed(mask, pio + PIO_IDR);
@@ -462,6 +491,67 @@ static void at91_mux_pio3_set_pulldown(void __iomem
*pio, unsigned mask, bool is
__raw_writel(mask, pio + (is_on ? PIO_PPDER : PIO_PPDDR));
}?
+static inline u32 read_drive_strength(void __iomem *reg, unsigned pin)
+{
+ return (__raw_readl(reg)
+ >> two_bit_pin_value_shift(pin)) & DRIVE_STRENGTH_MASK;
+}
+
+static unsigned at91_mux_sama5d3_get_drivestrength(void __iomem *pio,
unsigned pin)
+{
+ unsigned tmp = read_drive_strength(pio +
sama5d3_get_drive_register(pin), pin);
+
+ if (!tmp)
+ tmp = DRIVE_STRENGTH_LOW;
+
+ return tmp;
+}
+
+static unsigned at91_mux_sam9x5_get_drivestrength(void __iomem *pio,
unsigned pin)
+{
+ unsigned tmp = read_drive_strength(pio +
at91sam9x5_get_drive_register(pin), pin);
+
+ /*inverse settings*/
+ tmp = DRIVE_STRENGTH_HI - tmp;
+
+ return tmp;
+}
+
+static void set_drive_strength(void __iomem *reg, unsigned pin, u32
strength)
+{
+ unsigned tmp = __raw_readl(reg);
+
+ tmp &= ~(DRIVE_STRENGTH_MASK << two_bit_pin_value_shift(pin));
+ tmp |= strength << two_bit_pin_value_shift(pin);
+
+ __raw_writel(tmp, reg);
+}
+
+static void at91_mux_sama5d3_set_drivestrength(void __iomem *pio,
unsigned pin,
+ u32 setting)
+{
+ /* do nothing if setting is zero */
+ if (!setting)
+ return;
+
+ /* strength is 1 to 1 with setting for SAMA5 */
+ set_drive_strength(pio + sama5d3_get_drive_register(pin), pin, setting);
+}
+
+static void at91_mux_sam9x5_set_drivestrength(void __iomem *pio,
unsigned pin,
+ u32 setting)
+{
+ /* do nothing if setting is zero */
+ if (!setting)
+ return;
+
+ /* strength is inverse on SAM9s, 0 = hi, 1 = med, 2 = low, 3 = rsvd */
+ setting = DRIVE_STRENGTH_HI - setting;
+
+ set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin,
setting);
+}
+
+
static void at91_mux_pio3_disable_schmitt_trig(void __iomem *pio,
unsigned mask)
{
__raw_writel(__raw_readl(pio + PIO_SCHMITT) | mask, pio + PIO_SCHMITT);
@@ -491,6 +581,8 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
.set_deglitch = at91_mux_pio3_set_deglitch,
.get_debounce = at91_mux_pio3_get_debounce,
.set_debounce = at91_mux_pio3_set_debounce,
+ .get_drivestrength = at91_mux_sam9x5_get_drivestrength,
+ .set_drivestrength = at91_mux_sam9x5_set_drivestrength,
.get_pulldown = at91_mux_pio3_get_pulldown,
.set_pulldown = at91_mux_pio3_set_pulldown,
.get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
@@ -498,6 +590,26 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
.irq_type = alt_gpio_irq_type,
};
+static struct at91_pinctrl_mux_ops sama5d3_ops = {
+ .get_periph = at91_mux_pio3_get_periph,
+ .mux_A_periph = at91_mux_pio3_set_A_periph,
+ .mux_B_periph = at91_mux_pio3_set_B_periph,
+ .mux_C_periph = at91_mux_pio3_set_C_periph,
+ .mux_D_periph = at91_mux_pio3_set_D_periph,
+ .get_deglitch = at91_mux_pio3_get_deglitch,
+ .set_deglitch = at91_mux_pio3_set_deglitch,
+ .get_debounce = at91_mux_pio3_get_debounce,
+ .set_debounce = at91_mux_pio3_set_debounce,
+ .get_drivestrength = at91_mux_sama5d3_get_drivestrength,
+ .set_drivestrength = at91_mux_sama5d3_set_drivestrength,
+ .get_pulldown = at91_mux_pio3_get_pulldown,
+ .set_pulldown = at91_mux_pio3_set_pulldown,
+ .get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
+ .disable_schmitt_trig = at91_mux_pio3_disable_schmitt_trig,
+ .irq_type = alt_gpio_irq_type,
+};
+
+
static void at91_pin_dbg(const struct device *dev, const struct
at91_pmx_pin *pin)
{
if (pin->mux) {
@@ -736,6 +848,9 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
*config |= DEGLITCH;
if (info->ops->get_debounce && info->ops->get_debounce(pio, pin, &div))
*config |= DEBOUNCE | (div << DEBOUNCE_VAL_SHIFT);
+ if (info->ops->get_drivestrength)
+ *config |= DRIVE_STRENGTH | (info->ops->get_drivestrength(pio, pin)
+ << DRIVE_STRENGTH_SHIFT);
if (info->ops->get_pulldown && info->ops->get_pulldown(pio, pin))
*config |= PULL_DOWN;
if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
@@ -753,6 +868,7 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
void __iomem *pio;
int i;
unsigned long config;
+ unsigned pin;
for (i = 0; i < num_configs; i++) {
config = configs[i];
@@ -761,7 +877,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
"%s:%d, pin_id=%d, config=0x%lx",
__func__, __LINE__, pin_id, config);
pio = pin_to_controller(info, pin_to_bank(pin_id));
- mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
+ pin = pin_id % MAX_NB_GPIO_PER_BANK;
+ mask = pin_to_mask(pin);
if (config & PULL_UP && config & PULL_DOWN)
return -EINVAL;
@@ -773,6 +890,9 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
if (info->ops->set_debounce)
info->ops->set_debounce(pio, mask, config & DEBOUNCE,
(config & DEBOUNCE_VAL) >> DEBOUNCE_VAL_SHIFT);
+ if (info->ops->set_drivestrength)
+ info->ops->set_drivestrength(pio, pin,
+ (config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
if (info->ops->set_pulldown)
info->ops->set_pulldown(pio, mask, config & PULL_DOWN);
if (info->ops->disable_schmitt_trig && config & DIS_SCHMIT)
@@ -1551,6 +1671,7 @@ static void at91_gpio_probe_fixup(void)
static struct of_device_id at91_gpio_of_match[] = {
{ .compatible = "atmel,at91sam9x5-gpio", .data = &at91sam9x5_ops, },
+ { .compatible = "atmel,sama5d3-gpio", .data = &sama5d3_ops, },
{ .compatible = "atmel,at91rm9200-gpio", .data = &at91rm9200_ops },
{ /* sentinel */ }
};
diff --git a/include/dt-bindings/pinctrl/at91.h
b/include/dt-bindings/pinctrl/at91.h
index 0fee6ff..af71ebb 100644
--- a/include/dt-bindings/pinctrl/at91.h
+++ b/include/dt-bindings/pinctrl/at91.h
@@ -20,6 +20,11 @@
#define AT91_PINCTRL_PULL_UP_DEGLITCH (AT91_PINCTRL_PULL_UP |
AT91_PINCTRL_DEGLITCH)
+#define AT91_PINCTRL_DRIVE_STRENGTH_DEFAULT (0x0 << 5)
+#define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x1 << 5)
+#define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x2 << 5)
+#define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x3 << 5)
+
#define AT91_PIOA 0
#define AT91_PIOB 1
#define AT91_PIOC 2
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-21 5:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DC9A66.6080009@atmel.com>
On 11:39 Mon 20 Jan , Bo Shen wrote:
> Hi J,
>
> On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >On 10:59 Fri 17 Jan , Bo Shen wrote:
> >>In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
> >>is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
> >>
> >>Signed-off-by: Bo Shen <voice.shen@atmel.com>
> >>---
> >>
> >> drivers/usb/gadget/atmel_usba_udc.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> >>index 2cb52e0..7e67a81 100644
> >>--- a/drivers/usb/gadget/atmel_usba_udc.c
> >>+++ b/drivers/usb/gadget/atmel_usba_udc.c
> >>@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> >> if (ep_status) {
> >> int i;
> >>
> >>- for (i = 0; i < USBA_NR_ENDPOINTS; i++)
> >>+ for (i = 0; i < udc->num_ep; i++)
> >
> >no the limit need to specified in the driver as a checkpoint by the compatible
> >or platform driver id
>
> You mean, we should not trust the data passed from dt node or
> platform data? Or do you think we should do double confirm?
no base on the driver name or the compatible you will known the MAX EP
not based on the dt ep description
as we do on pinctrl-at91
Best Regards,
J.
> >> if (ep_status & (1 << i)) {
> >> if (ep_is_control(&udc->usba_ep[i]))
> >> usba_control_irq(udc, &udc->usba_ep[i]);
> >>--
> >>1.8.5.2
> >>
>
> Best Regards,
> Bo Shen
^ permalink raw reply
* [PATCH] ARM: asm: __und_usr_thumb need byteswap instructions in BE case
From: Victor Kamensky @ 2014-01-21 5:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DDD3C8.7080306@codeaurora.org>
On 20 January 2014 17:56, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 01/14/14 01:05, Dave Martin wrote:
>> On Mon, Jan 13, 2014 at 02:30:51PM -0800, Victor Kamensky wrote:
>>> __und_usr_thumb function deals with thumb2 opcodes. In case of BE
>>> image, it needs to byteswap half word thumb2 encoded instructions
>>> before further processing them.
>>>
>>> Without this fix BE image user-land thread executing first VFP
>>> instruction encoded in thumb2 fails with SIGILL, because kernel
>>> does not recognize instruction and does not enable VFP.
>>>
>>> Reported-by: Corey Melton <comelton@cisco.com>
>>> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
>> Good spot. This looks like the correct fix to me.
>>
>> Acked-by: Dave Martin <Dave.Martin@arm.com>
>>
>
> Can you please send this to the patch tracker? Feel free to add
>
> Tested-by: Stephen Boyd <sboyd@codeaurora.org>
Thanks. It is done on both counts. Patch is submitted as [1].
Thanks,
Victor
[1] http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7946/1
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>
^ permalink raw reply
* [PATCH REPOST 5/5] ARM: kvm MMIO support BE host running LE code
From: Anup Patel @ 2014-01-21 5:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAA3XUr23_Ea8feJNiqV_6ferDCiKVDiE8nvHKqS88tjBeckc9A@mail.gmail.com>
On Tue, Jan 21, 2014 at 10:54 AM, Victor Kamensky
<victor.kamensky@linaro.org> wrote:
> On 20 January 2014 17:19, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> On Mon, Jan 06, 2014 at 05:59:03PM -0800, Victor Kamensky wrote:
>>> On 6 January 2014 14:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>>> > On Mon, Jan 06, 2014 at 10:31:42PM +0000, Peter Maydell wrote:
>>> >> On 6 January 2014 18:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> >> > No matter how data is stored in memory (BE, LE, or
>>> >> > even PDP endianness), CPU registers always have a consistent
>>> >> > representation. They are immune to CPU endianness change, and storing
>>> >> > to/reading from memory won't change the value, as long as you use the
>>> >> > same endianness for writing/reading.
>>> >>
>>> >> Ah, endianness. This always confuses me, but I hope the following
>>> >> is correct... (in all the following when I say BE I mean BE8, not BE32,
>>> >> since BE32 and virtualization never occur in the same CPU).
>>> >>
>>> >> Certainly registers don't have endianness, but the entire point
>>> >> of the CPSR.E bit is exactly that it changes the value as it is
>>> >> stored to / read from memory, isn't it? -- that's where and when the
>>> >> byte-lane flipping happens.
>>> >>
>>> >> Where this impacts the hypervisor is that instead of actually sending
>>> >> the data out to the bus via the byte-swapping h/w, we've trapped instead.
>>> >> The hypervisor reads the original data directly from the guest CPU
>>> >> registers, and so it's the hypervisor and userspace support code that
>>> >> between them have to emulate the equivalent of the byte lane
>>> >> swapping h/w. You could argue that it shouldn't be the kernel's
>>> >> job, but since the kernel has to do it for the devices it emulates
>>> >> internally, I'm not sure that makes much sense.
>>> >
>>> > As far as I understand, this is exactly what vcpu_data_guest_to_host and
>>> > vcpu_data_host_to_guest do; emulate the byte lane swapping.
>>> >
>>> > The problem is that it only works on a little-endian host with the
>>> > current code, because be16_to_cpu (for example), actually perform a
>>> > byteswap, which is what needs to be emulated. On a big-endian host, we
>>> > do nothing, so we end up giving a byteswapped value to the emulated
>>> > device.
>>>
>>> Yes, that was my point on the thread: vcpu_data_guest_to_host and
>>> vcpu_data_host_to_guest functions for any given host endianity should
>>> give opposite endian results depending on CPSR E bit value. And
>>> currently it is not happening in BE host case. It seems that Peter and
>>> you agree with that and I gave example in another email with
>>> dynamically switching E bit illustrating this problem for BE host.
>>>
>>> > I think a cleaner fix than this patch is to just change the
>>> > be16_to_cpu() to a __swab16() instead, which clearly indicates that
>>> > 'here is the byte lane swapping'.
>>>
>>> Yes, that may work, but it is a bit orthogonal issue.
>>
>> Why? I don't think it is, I think it's addressing exactly the point at
>> hand.
>>
>>> And I don't think
>>> it is better. For this to work one need to change canonical endianity on
>>> one of the sides around vcpu_data_guest_to_host and
>>> vcpu_data_host_to_guest functions.
>>
>> You have to simply clearly define which format you want mmio.data to be
>> in.
>
> I believe it is already decided. 'mmio.data' in 'struct kvm_run' is not
> an integer type - it is bytes array. Bytes array does not have endianity.
> It is endian agnostic. Here is snippet from linux/kvm.h
>
> /* KVM_EXIT_MMIO */
> struct {
> __u64 phys_addr;
> __u8 data[8];
> __u32 len;
> __u8 is_write;
> } mmio;
>
> it is very natural to treat it as just a piece of memory. I.e when code reads
> emulated LE device address as integer, this array will contain integer
> placed in memory in LE order, data[3] is MSB, as it would be located in
> regular memory. When code reads emulated BE device address as
> integer this array will contain integer placed in memory in BE order,
> data[0] is MSB.
>
> You can think about it in that way: ARM system emulator runs on x86
> (LE) and on PPC (BE). How mmio.data array for the same emulated
> device should look like in across these two cases? I believe it should
> be identical - just a stream of bytes.
>
> Emulator code handles this situation quite nicely. For example check
> in qemu endianness field of MemoryRegionOps structure. Depending
> of the field value and current emulator endianity code will place
> results into 'mmio.data' array in right order. See [1] as an example
> in qemu where endianity of certain ARM devices were not declared
> correctly - it was marked as DEVICE_NATIVE_ENDIAN whereas
> it should be DEVICE_LITTLE_ENDIAN. After I changed that BE qemu
> pretty much started working. I strongly suspect if one would run
> ARM system emulation on PPC (BE) he/she would need the same
> changes.
>
> Note issue with virtio endianity is very different problem - there it
> is not clear for given arrangement of host/emulator how to treat
> virtio devices as LE or BE, and in what format data in rings
> descriptors are.
IMHO, device endianess should be taken care by device emulators only
because we can have Machine Model containing both LE devices and
BE devices. KVM ARM/ARM64 should only worry about endianess of
in-kernel emulated devices (e.g. VGIC). In general, QEMU or KVMTOOL
should be responsible of device endianess and for this QEMU or KVMTOOL
should also know whether Guest (or VM) is little-endian or big-endian.
Regards,
Anup
>
> Thanks,
> Victor
>
> [1] https://git.linaro.org/people/victor.kamensky/qemu-be.git/commitdiff/8599358f9711b7a546a2bba63b6277fbfb5b8e0c?hp=c4880f08ff9451e3d8020153e1a710ab4acee151
>
>> This is a user space interface across multiple architectures and
>> therefore something you have to consider carefully and you're limited in
>> choices to something that works with existing user space code.
>>
>>>
>>> Changing it on side that faces hypervisor (code that handles guest spilled
>>> CPU register set) does not make sense at all - if we will keep guest CPU
>>> register set in memory in LE form and hypervisor runs in BE (BE host),
>>> code that spills registers would need to do constant byteswaps. Also any
>>> access by host kernel and hypervisor (all running in BE) would need to do
>>> byteswaps while working with guest saved registers.
>>>
>>> Changing canonical form of data on side that faces emulator and mmio
>>> part of kvm_run does not make sense either. kvm_run mmio.data field is
>>> bytes array, when it comes to host kernel from emulator, it already contains
>>> device memory in correct endian order that corresponds to endianity of
>>> emulated device. For example for LE device word read access, after call is
>>> emulated, mmio.data will contain mmio.data[0], mmio.data[1], mmio.data[2]
>>> mmio.data[3] values in LE order (mmio.data[3] is MSB). Now look at
>>> mmio_read_buf function introduced by Marc's 6d89d2d9 commit, this function
>>> will byte copy this mmio.data buffer into integer according to ongoing mmio
>>> access size. Note in BE host case such integer, in 'data' variable of
>>> kvm_handle_mmio_return function, will have byteswapped value. Now when it will
>>> be passed into vcpu_data_host_to_guest function, and it emulates read access
>>> of guest with E bit set, and if we follow your suggestion, it will be
>>> byteswapped.
>>> I.e 'data' integer will contain non byteswapped value of LE device. It will be
>>> further stored into some vcpu_reg register, still in native format (BE
>>> store), and
>>> further restored into guest CPU register, still non byteswapped (BE hypervisor).
>>> And that is not what BE client reading word of LE device expects - BE client
>>> knowing that it reads LE device with E bit set, it will issue additional rev
>>> instruction to get device memory as integer. If we really want to follow your
>>> suggestion, one may introduce compensatory byteswaps in mmio_read_buf
>>> and mmio_write_buf functions in case of BE host, rather then just do
>>> memcpy ... but I am not sure what it will buy us - in BE case it will swap data
>>> twice.
>>>
>>> Note in above description by "canonical" I mean some form of data regardless
>>> of current access CPSR E value. But it may differ depending on host endianess.
>>>
>>
>> There's a lot of text to digest here, talking about a canonical form
>> here doesn't help; just define the layout of the destination byte array.
>> I also got completely lost in what you're referring to when you talk
>> about 'sides' here.
>>
>> The thing we must decide is how the data is stored in
>> kvm_exit_mmio.data. See Peter's recent thread "KVM and
>> variable-endianness guest CPUs". Once we agree on this, the rest should
>> be easy (assuming we use the same structure for the data in the kernel's
>> internal kvm_exit_mmio declared on the stack in io_mem_abort()).
>>
>> The format you suggest requires any consumer of this data to consider
>> the host endianness, which I don't think makes anything more clear (see
>> my comment on the vgic patch).
>>
>> The in-kernel interface between the io_mem_abort() code and any
>> in-kernel emulated device must do exactly the same as the interface
>> between KVM and QEMU must do for KVM_EXIT_MMIO.
>>
>> --
>> Christoffer
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
^ permalink raw reply
* [PATCH REPOST 5/5] ARM: kvm MMIO support BE host running LE code
From: Victor Kamensky @ 2014-01-21 5:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121011915.GM13432@cbox>
On 20 January 2014 17:19, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Mon, Jan 06, 2014 at 05:59:03PM -0800, Victor Kamensky wrote:
>> On 6 January 2014 14:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> > On Mon, Jan 06, 2014 at 10:31:42PM +0000, Peter Maydell wrote:
>> >> On 6 January 2014 18:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> >> > No matter how data is stored in memory (BE, LE, or
>> >> > even PDP endianness), CPU registers always have a consistent
>> >> > representation. They are immune to CPU endianness change, and storing
>> >> > to/reading from memory won't change the value, as long as you use the
>> >> > same endianness for writing/reading.
>> >>
>> >> Ah, endianness. This always confuses me, but I hope the following
>> >> is correct... (in all the following when I say BE I mean BE8, not BE32,
>> >> since BE32 and virtualization never occur in the same CPU).
>> >>
>> >> Certainly registers don't have endianness, but the entire point
>> >> of the CPSR.E bit is exactly that it changes the value as it is
>> >> stored to / read from memory, isn't it? -- that's where and when the
>> >> byte-lane flipping happens.
>> >>
>> >> Where this impacts the hypervisor is that instead of actually sending
>> >> the data out to the bus via the byte-swapping h/w, we've trapped instead.
>> >> The hypervisor reads the original data directly from the guest CPU
>> >> registers, and so it's the hypervisor and userspace support code that
>> >> between them have to emulate the equivalent of the byte lane
>> >> swapping h/w. You could argue that it shouldn't be the kernel's
>> >> job, but since the kernel has to do it for the devices it emulates
>> >> internally, I'm not sure that makes much sense.
>> >
>> > As far as I understand, this is exactly what vcpu_data_guest_to_host and
>> > vcpu_data_host_to_guest do; emulate the byte lane swapping.
>> >
>> > The problem is that it only works on a little-endian host with the
>> > current code, because be16_to_cpu (for example), actually perform a
>> > byteswap, which is what needs to be emulated. On a big-endian host, we
>> > do nothing, so we end up giving a byteswapped value to the emulated
>> > device.
>>
>> Yes, that was my point on the thread: vcpu_data_guest_to_host and
>> vcpu_data_host_to_guest functions for any given host endianity should
>> give opposite endian results depending on CPSR E bit value. And
>> currently it is not happening in BE host case. It seems that Peter and
>> you agree with that and I gave example in another email with
>> dynamically switching E bit illustrating this problem for BE host.
>>
>> > I think a cleaner fix than this patch is to just change the
>> > be16_to_cpu() to a __swab16() instead, which clearly indicates that
>> > 'here is the byte lane swapping'.
>>
>> Yes, that may work, but it is a bit orthogonal issue.
>
> Why? I don't think it is, I think it's addressing exactly the point at
> hand.
>
>> And I don't think
>> it is better. For this to work one need to change canonical endianity on
>> one of the sides around vcpu_data_guest_to_host and
>> vcpu_data_host_to_guest functions.
>
> You have to simply clearly define which format you want mmio.data to be
> in.
I believe it is already decided. 'mmio.data' in 'struct kvm_run' is not
an integer type - it is bytes array. Bytes array does not have endianity.
It is endian agnostic. Here is snippet from linux/kvm.h
/* KVM_EXIT_MMIO */
struct {
__u64 phys_addr;
__u8 data[8];
__u32 len;
__u8 is_write;
} mmio;
it is very natural to treat it as just a piece of memory. I.e when code reads
emulated LE device address as integer, this array will contain integer
placed in memory in LE order, data[3] is MSB, as it would be located in
regular memory. When code reads emulated BE device address as
integer this array will contain integer placed in memory in BE order,
data[0] is MSB.
You can think about it in that way: ARM system emulator runs on x86
(LE) and on PPC (BE). How mmio.data array for the same emulated
device should look like in across these two cases? I believe it should
be identical - just a stream of bytes.
Emulator code handles this situation quite nicely. For example check
in qemu endianness field of MemoryRegionOps structure. Depending
of the field value and current emulator endianity code will place
results into 'mmio.data' array in right order. See [1] as an example
in qemu where endianity of certain ARM devices were not declared
correctly - it was marked as DEVICE_NATIVE_ENDIAN whereas
it should be DEVICE_LITTLE_ENDIAN. After I changed that BE qemu
pretty much started working. I strongly suspect if one would run
ARM system emulation on PPC (BE) he/she would need the same
changes.
Note issue with virtio endianity is very different problem - there it
is not clear for given arrangement of host/emulator how to treat
virtio devices as LE or BE, and in what format data in rings
descriptors are.
Thanks,
Victor
[1] https://git.linaro.org/people/victor.kamensky/qemu-be.git/commitdiff/8599358f9711b7a546a2bba63b6277fbfb5b8e0c?hp=c4880f08ff9451e3d8020153e1a710ab4acee151
> This is a user space interface across multiple architectures and
> therefore something you have to consider carefully and you're limited in
> choices to something that works with existing user space code.
>
>>
>> Changing it on side that faces hypervisor (code that handles guest spilled
>> CPU register set) does not make sense at all - if we will keep guest CPU
>> register set in memory in LE form and hypervisor runs in BE (BE host),
>> code that spills registers would need to do constant byteswaps. Also any
>> access by host kernel and hypervisor (all running in BE) would need to do
>> byteswaps while working with guest saved registers.
>>
>> Changing canonical form of data on side that faces emulator and mmio
>> part of kvm_run does not make sense either. kvm_run mmio.data field is
>> bytes array, when it comes to host kernel from emulator, it already contains
>> device memory in correct endian order that corresponds to endianity of
>> emulated device. For example for LE device word read access, after call is
>> emulated, mmio.data will contain mmio.data[0], mmio.data[1], mmio.data[2]
>> mmio.data[3] values in LE order (mmio.data[3] is MSB). Now look at
>> mmio_read_buf function introduced by Marc's 6d89d2d9 commit, this function
>> will byte copy this mmio.data buffer into integer according to ongoing mmio
>> access size. Note in BE host case such integer, in 'data' variable of
>> kvm_handle_mmio_return function, will have byteswapped value. Now when it will
>> be passed into vcpu_data_host_to_guest function, and it emulates read access
>> of guest with E bit set, and if we follow your suggestion, it will be
>> byteswapped.
>> I.e 'data' integer will contain non byteswapped value of LE device. It will be
>> further stored into some vcpu_reg register, still in native format (BE
>> store), and
>> further restored into guest CPU register, still non byteswapped (BE hypervisor).
>> And that is not what BE client reading word of LE device expects - BE client
>> knowing that it reads LE device with E bit set, it will issue additional rev
>> instruction to get device memory as integer. If we really want to follow your
>> suggestion, one may introduce compensatory byteswaps in mmio_read_buf
>> and mmio_write_buf functions in case of BE host, rather then just do
>> memcpy ... but I am not sure what it will buy us - in BE case it will swap data
>> twice.
>>
>> Note in above description by "canonical" I mean some form of data regardless
>> of current access CPSR E value. But it may differ depending on host endianess.
>>
>
> There's a lot of text to digest here, talking about a canonical form
> here doesn't help; just define the layout of the destination byte array.
> I also got completely lost in what you're referring to when you talk
> about 'sides' here.
>
> The thing we must decide is how the data is stored in
> kvm_exit_mmio.data. See Peter's recent thread "KVM and
> variable-endianness guest CPUs". Once we agree on this, the rest should
> be easy (assuming we use the same structure for the data in the kernel's
> internal kvm_exit_mmio declared on the stack in io_mem_abort()).
>
> The format you suggest requires any consumer of this data to consider
> the host endianness, which I don't think makes anything more clear (see
> my comment on the vgic patch).
>
> The in-kernel interface between the io_mem_abort() code and any
> in-kernel emulated device must do exactly the same as the interface
> between KVM and QEMU must do for KVM_EXIT_MMIO.
>
> --
> Christoffer
^ permalink raw reply
* No subject
From: John Tobias @ 2014-01-21 4:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi Guys,
Just wondering if you encountered the error below and if there's any
existing patch?.
Regards,
john
[ 860.354231] Unable to handle kernel paging request at virtual
address eaffff9d
[ 860.361466] pgd = bf190000
[ 860.364177] [eaffff9d] *pgd=00000000
[ 860.367773] Internal error: Oops: 5 [#1] ARM
[ 860.372048] Modules linked in: bt8xxx(O) sd8xxx(O) mlan(O)
[ 860.377597] CPU: 0 PID: 82 Comm: ksdioirqd/mmc1 Tainted: G
O 3.13.0-rc1 #1
[ 860.385344] task: bf05c280 ti: bf01a000 task.ti: bf01a000
[ 860.390756] PC is at mmc_io_rw_extended+0x38/0x310
[ 860.395552] LR is at mmc_io_rw_extended+0x38/0x310
[ 860.400346] pc : [<80319b68>] lr : [<80319b68>] psr: 400f0013
[ 860.400346] sp : bf01bc90 ip : bf01bd60 fp : bf01bd8c
[ 860.411824] r10: 806f9748 r9 : bf9e7800 r8 : eb075fb4
[ 860.417051] r7 : eaffff9d r6 : 80327c24 r5 : 1a000009 r4 : 00000007
[ 860.423580] r3 : 00000000 r2 : ffffffc4 r1 : 00000000 r0 : bf01bd1c
[ 860.430112] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment kernel
[ 860.437423] Control: 10c53c7d Table: bf190059 DAC: 00000015
[ 860.443172] Process ksdioirqd/mmc1 (pid: 82, stack limit = 0xbf01a238)
[ 860.449702] Stack: (0xbf01bc90 to 0xbf01c000)
[ 860.454065] bc80: 80319e04
00000440 00000139 00000000
[ 860.462246] bca0: 00000139 00000000 817de8c2 000000c0 00000700
beac60c0 3b9aca00 00000000
[ 860.470427] bcc0: 00000100 00000007 00000000 00000200 00000700
00000000 bf01bd1c 00000001
[ 860.478609] bce0: bf01bca8 00000000 00000035 1a001207 00002000
00000000 00000000 00000000
[ 860.486790] bd00: 000001b5 00000000 00000000 00000000 00000000
bf01bcb8 bf01bd1c 00000000
[ 860.494971] bd20: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 860.503153] bd40: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 860.511334] bd60: bf01bd94 bf01bd70 80324254 80327c24 bf01be04
bf9e7800 806f9748 b600003f
[ 860.519515] bd80: bf01bdd4 bf01bd90 8031ade8 80319b3c bf01be04
80327c24 00000007 1a000009
[ 860.527697] bda0: bf05c280 000001ff 00100100 bfb52000 00000100
00000007 beac60c0 00010009
[ 860.535878] bdc0: 7f03e578 c0adb000 bf01bdec bf01bdd8 8031aef8
8031ad10 beac60c0 00000700
[ 860.544059] bde0: bf01be14 bf01bdf0 7f08c2bc 8031aee0 00000000
00000009 00000700 7f03d328
[ 860.552240] be00: bfad9a00 00000000 bf01be24 bf01be18 7f0622f0
7f08c278 bf01be6c bf01be28
[ 860.560422] be20: 7f0137a8 7f0622ec bf01be44 bf01be38 8004efd4
00000001 bf01be5c 00000000
[ 860.568603] be40: 804fc960 c0adb000 c0adbce4 7f03d328 00000000
7f03e578 7f061dec bfb52000
[ 860.576784] be60: bf01beac bf01be70 7f001e68 7f0134f8 00000000
00000000 00000000 00000000
[ 860.584965] be80: 00000000 bfb52000 7f09a0d4 bfb51800 00000000
bf9e7800 bf01a000 00000001
[ 860.593146] bea0: bf01bec4 bf01beb0 7f05a138 7f001ce0 00000001
00000000 bf01bed4 bf01bec8
[ 860.601327] bec0: 7f08bfe4 7f05a0cc bf01bf24 bf01bed8 8031ba0c
7f08bfc4 00000000 bf01bef3
[ 860.609509] bee0: bfb51800 00000001 bf9e7bb4 7fffffff 0215d9c0
00000001 8031b874 00000000
[ 860.617690] bf00: bf15d9c0 bf9e7800 8031b874 00000000 00000000
00000000 bf01bfac bf01bf28
[ 860.625871] bf20: 8003c7e8 8031b880 bf01bf44 00000000 8004efd4
bf9e7800 00000000 00000001
[ 860.634052] bf40: dead4ead ffffffff ffffffff 80702208 00000000
00000000 80600be8 bf01bf5c
[ 860.642233] bf60: bf01bf5c 00000000 00000001 dead4ead ffffffff
ffffffff 80702208 00000000
[ 860.650415] bf80: 00000000 80600be8 bf01bf88 bf01bf88 bf15d9c0
8003c724 00000000 00000000
[ 860.658595] bfa0: 00000000 bf01bfb0 8000f308 8003c730 00000000
00000000 00000000 00000000
[ 860.666776] bfc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 860.674956] bfe0: 00000000 00000000 00000000 00000000 00000013
00000000 00000000 00000000
[ 860.683133] Backtrace:
[ 860.685607] [<80319b30>] (mmc_io_rw_extended+0x0/0x310) from
[<8031ade8>] (sdio_io_rw_ext_helper+0xe4/0x1a4)
[ 860.695442] [<8031ad04>] (sdio_io_rw_ext_helper+0x0/0x1a4) from
[<8031aef8>] (sdio_readsb+0x24/0x2c)
[ 860.704676] [<8031aed4>] (sdio_readsb+0x0/0x2c) from [<7f08c2bc>]
(woal_read_data_sync+0x50/0x8c [sd8xxx])
[ 860.714455] [<7f08c26c>] (woal_read_data_sync+0x0/0x8c [sd8xxx])
from [<7f0622f0>] (moal_read_data_sync+0x10/0x14 [sd8xxx])
[ 860.725586] r8:00000000 r7:bfad9a00 r6:7f03d328 r5:00000700 r4:00000009
r3:00000000
[ 860.733603] [<7f0622e0>] (moal_read_data_sync+0x0/0x14 [sd8xxx])
from [<7f0137a8>] (wlan_process_int_status+0x2bc/0x928 [mlan])
[ 860.745139] [<7f0134ec>] (wlan_process_int_status+0x0/0x928 [mlan])
from [<7f001e68>] (mlan_main_process+0x194/0x750 [mlan])
[ 860.756436] [<7f001cd4>] (mlan_main_process+0x0/0x750 [mlan]) from
[<7f05a138>] (woal_interrupt+0x78/0x94 [sd8xxx])
[ 860.766982] [<7f05a0c0>] (woal_interrupt+0x0/0x94 [sd8xxx]) from
[<7f08bfe4>] (woal_sdio_interrupt+0x2c/0x30 [sd8xxx])
[ 860.777677] r5:00000000 r4:00000001
[ 860.781351] [<7f08bfb8>] (woal_sdio_interrupt+0x0/0x30 [sd8xxx])
from [<8031ba0c>] (sdio_irq_thread+0x198/0x368)
[ 860.791537] [<8031b874>] (sdio_irq_thread+0x0/0x368) from
[<8003c7e8>] (kthread+0xc4/0xe0)
[ 860.799814] [<8003c724>] (kthread+0x0/0xe0) from [<8000f308>]
(ret_from_fork+0x14/0x2c)
[ 860.807818] r7:00000000 r6:00000000 r5:8003c724 r4:bf15d9c0
[ 860.813538] Code: e1a09003 e59b400c e59b5010 ebfc3a75 (e5973000)
[ 860.819646] ---[ end trace 1920724507aeb8b4 ]---
[ 860.824270] Kernel panic - not syncing: Fatal exception
^ permalink raw reply
* pinctrl: at91: drive strength control
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-21 4:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJjB1qKAeMOdP03v85ZPcvxvS96A+_-HzO0565wtaKK8278guw@mail.gmail.com>
On 22:34 Mon 20 Jan , Mark Roszko wrote:
> If I choose it at runtime what's the best way for me to detect the SoC
> type to choose the correct register address at runtime? Only way I see
> is adding new compatible types for the SAMA5s and the SAM9s with the
> registers.
>
we could do it via cpu_is as a check as we may have older dts
but it's better to do it via compatible
> On Mon, Jan 20, 2014 at 9:19 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 20:07 Mon 20 Jan , Marek Roszko wrote:
> >> Hello Nick and Jean-Christophe,
> >>
> >> I submit a idea/patch for comments. The current pintctrl driver is missing the ability to set the drive strength in the SAMA5D3s and a few of the SAM9s.
> >> A little feature I myself need for the SAMA5D3s because somebody let an analog engineer do the hardware design.
> >>
> >> Issues with this patch I need help with:
> >> 1. Atmel for some reason shifted the PIO_DRIVER1 and PIO_DRIVER2 registers by one register address between the SAM9 chips and the SAMA5D3s
> >>
> >> i.e. this is the SAM9s
> >> #define PIO_DRIVER1_V1 0x114
> >> #define PIO_DRIVER2_V1 0x118
> >>
> >> this is the SAMA5D3s
> >> #define PIO_DRIVER1_V2 0x118
> >> #define PIO_DRIVER2_V2 0x11C
> >>
> >>
> >> 2. Atmel changed the meaning of value of "low", "medium" and "high" drive strengths between the two sets of chips as well.
> >>
> >> SAM9s do:
> >> 00 = high
> >> 01 = medium
> >> 10 = low
> >> 11 = reserved/undefined
> >>
> >> SAMA5D3s do:
> >> 00 = low
> >> 01 = low
> >> 10 = medium
> >> 11 = high
> >>
> >> 3. The SAM9G25, SAM9G35 have the PIO_DRIVER but the SAM9G45 does not have the PIO_DRIVER register as an example of
> >> how oddly the behavior doesn't appear everywhere. This could also be because the datasheets for the G45 and G46 are
> >> not updated to the new style while the others already have been?
> >>
> >> Side note: The SAMA5D3 datasheet says the default drive strength is "low" with register values 0x00000000,
> >> in reality the default is "medium" with 0xAAAAAAAA as the default value.
> >> Confirmed with the JLINK debugger and by Atmel support. Not an real issue, just a note when testing.
> >
> > check the coding style tab for indentatnion, 80 charrs per line max etc...
> >>
> >>
> >> So the only simple way I see is to #ifdef the SOC type/chip.
> >> There's probably better far better ways I don't know.
> >>
> >> ---
> >> arch/arm/mach-at91/include/mach/at91_pio.h | 9 +++++
> >> drivers/pinctrl/pinctrl-at91.c | 54 +++++++++++++++++++++++++++-
> >> include/dt-bindings/pinctrl/at91.h | 13 +++++++
> >> 3 files changed, 75 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/mach-at91/include/mach/at91_pio.h b/arch/arm/mach-at91/include/mach/at91_pio.h
> >> index 732b11c..28e8801 100644
> >> --- a/arch/arm/mach-at91/include/mach/at91_pio.h
> >> +++ b/arch/arm/mach-at91/include/mach/at91_pio.h
> >> @@ -66,6 +66,15 @@
> >> #define PIO_FRLHSR 0xd8 /* Fall/Rise - Low/High Status Register */
> >> #define PIO_SCHMITT 0x100 /* Schmitt Trigger Register */
> >>
> >> +/*
> >> + * SoC Specific PIO Address Offsets
> >> + */
> >> +#define PIO_DRIVER1_V1 0x114
> >> +#define PIO_DRIVER2_V1 0x118
> >> +
> >> +#define PIO_DRIVER1_V2 0x118
> >> +#define PIO_DRIVER2_V2 0x11C
> >> +
> >> #define ABCDSR_PERIPH_A 0x0
> >> #define ABCDSR_PERIPH_B 0x1
> >> #define ABCDSR_PERIPH_C 0x2
> >> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >> index a7549c4..ccf456e 100644
> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> @@ -62,10 +62,37 @@ static int gpio_banks;
> >> #define DEGLITCH (1 << 2)
> >> #define PULL_DOWN (1 << 3)
> >> #define DIS_SCHMIT (1 << 4)
> >> +#define SET_DRIVE_STRENGTH (1 << 5)
> >> +#define DRIVE_STRENGTH_SHIFT 6
> >> +#define DRIVE_STRENGTH (0x3 << DRIVE_STRENGTH_SHIFT)
> >> #define DEBOUNCE (1 << 16)
> >> #define DEBOUNCE_VAL_SHIFT 17
> >> #define DEBOUNCE_VAL (0x3fff << DEBOUNCE_VAL_SHIFT)
> >>
> >> +#define DRIVE_STRENGTH_MASK 0x3
> >> +
> >> +#define NUM_PINS_PER_DRIVE_STRENGTH_REG 16
> >> +
> >> +#define TWO_BIT_PIN_TO_SHIFT(pin)\
> >> + (2*((pin >= NUM_PINS_PER_DRIVE_STRENGTH_REG) ? \
> >> + pin - NUM_PINS_PER_DRIVE_STRENGTH_REG : pin))
> >
> >> +
> >> +#define TWO_BIT_PIN_TO_MASK(pin)\
> >> + (DRIVE_STRENGTH_MASK << TWO_BIT_PIN_TO_SHIFT(pin))
> >> +
> >> +#if defined(CONFIG_SOC_SAMA5D3)
> >> + #define PIO_DRIVER1 PIO_DRIVER1_V2 /* Drive Strength Register 1 */
> >> + #define PIO_DRIVER2 PIO_DRIVER2_V2 /* Drive Strength Register 2 */
> >> +#elif defined(CONFIG_SOC_AT91SAM9X5) || defined(SOC_AT91SAM9G35) || defined(SOC_AT91SAM9G25)
> >> + #define PIO_DRIVER1 PIO_DRIVER1_V1 /* Drive Strength Register 1 */
> >> + #define PIO_DRIVER2 PIO_DRIVER2_V1 /* Drive Strength Register 2 */
> >> +#endif
> > no ifdef this is choose at runtime
> >> +
> >> +#if defined(PIO_DRIVER1) && defined(PIO_DRIVER2)
> >> + #define PIO_DRIVER(pin)\
> >> + ((pin > NUM_PINS_PER_DRIVE_STRENGTH_REG-1) ? PIO_DRIVER2 : PIO_DRIVER1)
> >> +#endif
> > use inline function
> >> +
> >> /**
> >> * struct at91_pmx_func - describes AT91 pinmux functions
> >> * @name: the name of this specific function
> >> @@ -148,6 +175,8 @@ struct at91_pinctrl_mux_ops {
> >> void (*set_deglitch)(void __iomem *pio, unsigned mask, bool is_on);
> >> bool (*get_debounce)(void __iomem *pio, unsigned pin, u32 *div);
> >> void (*set_debounce)(void __iomem *pio, unsigned mask, bool is_on, u32 div);
> >> + int (*get_drivestrength)(void __iomem *pio, unsigned pin);
> >> + void (*set_drivestrength)(void __iomem *pio, unsigned pin, u32 strength);
> >> bool (*get_pulldown)(void __iomem *pio, unsigned pin);
> >> void (*set_pulldown)(void __iomem *pio, unsigned mask, bool is_on);
> >> bool (*get_schmitt_trig)(void __iomem *pio, unsigned pin);
> >> @@ -462,6 +491,20 @@ static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is
> >> __raw_writel(mask, pio + (is_on ? PIO_PPDER : PIO_PPDDR));
> >> }
> >>
> >> +static int at91_mux_pio3_get_drivestrength(void __iomem *pio, unsigned pin)
> >> +{
> >> + return (__raw_readl(pio + PIO_DRIVER(pin)) >> TWO_BIT_PIN_TO_SHIFT(pin)) & DRIVE_STRENGTH_MASK;
> >> +}
> >> +
> >> +static void at91_mux_pio3_set_drivestrength(void __iomem *pio, unsigned pin, u32 strength)
> >> +{
> >> + unsigned tmp = __raw_readl(pio + PIO_DRIVER(pin));
> >> + tmp &= ~TWO_BIT_PIN_TO_MASK(pin);
> >> + tmp |= strength << TWO_BIT_PIN_TO_SHIFT(pin);
> >> +
> >> + __raw_writel(tmp, pio + PIO_DRIVER(pin));
> >> +}
> >> +
> >> static void at91_mux_pio3_disable_schmitt_trig(void __iomem *pio, unsigned mask)
> >> {
> >> __raw_writel(__raw_readl(pio + PIO_SCHMITT) | mask, pio + PIO_SCHMITT);
> >> @@ -491,6 +534,8 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
> >> .set_deglitch = at91_mux_pio3_set_deglitch,
> >> .get_debounce = at91_mux_pio3_get_debounce,
> >> .set_debounce = at91_mux_pio3_set_debounce,
> >> + .get_drivestrength = at91_mux_pio3_get_drivestrength,
> >> + .set_drivestrength = at91_mux_pio3_set_drivestrength,
> >> .get_pulldown = at91_mux_pio3_get_pulldown,
> >> .set_pulldown = at91_mux_pio3_set_pulldown,
> >> .get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
> >> @@ -736,6 +781,8 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> >> *config |= DEGLITCH;
> >> if (info->ops->get_debounce && info->ops->get_debounce(pio, pin, &div))
> >> *config |= DEBOUNCE | (div << DEBOUNCE_VAL_SHIFT);
> >> + if (info->ops->get_drivestrength)
> >> + *config |= DRIVE_STRENGTH | (info->ops->get_drivestrength(pio, pin) << DRIVE_STRENGTH_SHIFT);
> >> if (info->ops->get_pulldown && info->ops->get_pulldown(pio, pin))
> >> *config |= PULL_DOWN;
> >> if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
> >> @@ -753,6 +800,7 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> >> void __iomem *pio;
> >> int i;
> >> unsigned long config;
> >> + unsigned pin;
> >>
> >> for (i = 0; i < num_configs; i++) {
> >> config = configs[i];
> >> @@ -761,7 +809,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> >> "%s:%d, pin_id=%d, config=0x%lx",
> >> __func__, __LINE__, pin_id, config);
> >> pio = pin_to_controller(info, pin_to_bank(pin_id));
> >> - mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
> >> + pin = pin_id % MAX_NB_GPIO_PER_BANK;
> >> + mask = pin_to_mask(pin);
> >>
> >> if (config & PULL_UP && config & PULL_DOWN)
> >> return -EINVAL;
> >> @@ -773,6 +822,9 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> >> if (info->ops->set_debounce)
> >> info->ops->set_debounce(pio, mask, config & DEBOUNCE,
> >> (config & DEBOUNCE_VAL) >> DEBOUNCE_VAL_SHIFT);
> >> + if (info->ops->set_drivestrength && config & SET_DRIVE_STRENGTH)
> >> + info->ops->set_drivestrength(pio, pin,
> >> + (config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
> >> if (info->ops->set_pulldown)
> >> info->ops->set_pulldown(pio, mask, config & PULL_DOWN);
> >> if (info->ops->disable_schmitt_trig && config & DIS_SCHMIT)
> >> diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
> >> index 0fee6ff..78621a4 100644
> >> --- a/include/dt-bindings/pinctrl/at91.h
> >> +++ b/include/dt-bindings/pinctrl/at91.h
> >> @@ -20,6 +20,19 @@
> >>
> >> #define AT91_PINCTRL_PULL_UP_DEGLITCH (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DEGLITCH)
> >>
> >> +#define AT91_PINCTRL_SET_DRIVE_STRENGTH (1 << 5)
> >> +
> >> +#if defined(CONFIG_SOC_SAMA5D3)
> >> + #define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x2 << 6)
> >> + #define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x1 << 6)
> >> + #define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x0 << 6)
> >> +#elif defined(CONFIG_SOC_AT91SAM9X5) || defined(SOC_AT91SAM9G35) || defined(SOC_AT91SAM9G25)
> >> + #define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x1 << 6)
> >> + #define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x2 << 6)
> >> + #define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x3 << 6)
> >> +#endif
> >
> > no only one binding we do not care of the IP details
> >
> > 0 means no drive strength specified
> >> +
> >> +
> >> #define AT91_PIOA 0
> >> #define AT91_PIOB 1
> >> #define AT91_PIOC 2
> >> --
> >> 1.7.10.4
> >>
> >>
> >>
> >>
> >> --
> >> Mark
> >>
> >>
>
>
>
> --
> Mark
^ permalink raw reply
* [PATCH 06/20] ARM64 / ACPI: Introduce some PCI functions when PCI is enabled
From: Hanjun Guo @ 2014-01-21 3:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201401201939.53035.arnd@arndb.de>
On 2014-1-21 2:39, Arnd Bergmann wrote:
> On Monday 20 January 2014, Hanjun Guo wrote:
>>>> acpi_register_ioapic()/acpi_unregister_ioapic() will be used for IOAPIC
>>>> hotplug and GIC distributor is something like IOAPIC on x86, so I think
>>>> these two functions can be reserved for future use.
>>> But GIC is not hotplugged, is it? It still sounds x86 specific to me.
>>
>> Well, if we want to do physical CPU hotplug on ARM/ARM64 (maybe years
>> later?),
>> then GIC add/remove is needed because we have to remove GIC
>> on the SoC too when we remove the physical CPU.
>
> In general, I recommend not planning for the future in kernel code when you
> don't know what is going to happen. It's always easy enough to change
> things once you get there, as long as no stable ABI is involved.
Ok, I agree with you.
>
> I just looked at the caller of these functions, and found a self-contained
> PCI driver in drivers/pci/ioapic.c, which uses two sepate PCI classes for
> ioapic and ioxapic. I think it's a safe assumption to say that even if we
> get ARM CPU+GIC hotplug, that would not use the same ioapic driver. This
> driver is currently marked X86-only, and that should probably stay this way,
> so you won't need the hooks.
Will find a suitable way to fix that in next version, thanks for you comments :)
Hanjun
^ permalink raw reply
* [PATCH 1/3] ACPI / idle: Move idle_boot_override out of the arch directory
From: Hanjun Guo @ 2014-01-21 3:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1945528.uEVBjXteWy@vostro.rjw.lan>
On 2014-1-21 7:34, Rafael J. Wysocki wrote:
> On Monday, January 20, 2014 10:08:41 PM Hanjun Guo wrote:
>> On 2014?01?18? 21:47, Rafael J. Wysocki wrote:
>>> On Saturday, January 18, 2014 11:52:18 AM Hanjun Guo wrote:
>>>> On 2014-1-18 11:45, Hanjun Guo wrote:
>>>>> On 2014-1-17 20:06, Sudeep Holla wrote:
>>>>>> On 17/01/14 02:03, Hanjun Guo wrote:
>>>>>>> Move idle_boot_override out of the arch directory to be a single enum
>>>>>>> including both platforms values, this will make it rather easier to
>>>>>>> avoid ifdefs around which definitions are for which processor in
>>>>>>> generally used ACPI code.
>>>>>>>
>>>>>>> IDLE_FORCE_MWAIT for IA64 is not used anywhere, so romove it.
>>>>>>>
>>>>>>> No functional change in this patch.
>>>>>>>
>>>>>>> Suggested-by: Alan <gnomes@lxorguk.ukuu.org.uk>
>>>>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>>>>>> ---
>>>> [...]
>>>>>>> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
>>>>>>> index 03e235ad..e324561 100644
>>>>>>> --- a/include/linux/cpu.h
>>>>>>> +++ b/include/linux/cpu.h
>>>>>>> @@ -220,6 +220,14 @@ void cpu_idle(void);
>>>>>>>
>>>>>>> void cpu_idle_poll_ctrl(bool enable);
>>>>>>>
>>>>>>> +enum idle_boot_override {
>>>>>>> + IDLE_NO_OVERRIDE = 0,
>>>>>>> + IDLE_HALT,
>>>>>>> + IDLE_NOMWAIT,
>>>>>>> + IDLE_POLL,
>>>>>>> + IDLE_POWERSAVE_OFF
>>>>>>> +};
>>>>>>> +
>>>>>> I do understand the idea behind this change, but IMO HALT and MWAIT are x86
>>>>>> specific and may not make sense for other architectures.
>>>>> yes, this is the strange part, the value is arch-dependent.
>>>>>
>>>>>> It will also require every architecture using ACPI to export
>>>>>> boot_option_idle_override which may not be really required.
>>>>> so, how about forget this patch and move boot_option_idle_override
>>>>> related code into arch directory such as arch/x86/acpi/boot.c for
>>>>> x86?
>>>> The general idea is that we can move all the arch-dependent codes
>>>> in ACPI driver to arch directory, then make codes in drivers/acpi/
>>>> arch independent.
>>> Well, MWAIT is arch-dependent, so I'm not sure how IDLE_NOMWAIT fits into
>>> include/linux/cpu.h?
>>
>> So you will not happy with this patch and should find another solution?
>
> No, I'm not happy with it.
>
> If you want to move that to an arch-agnostic header, the symbol names cannot
> be arch-dependent any more.
Ok, will find another solution for that, thanks for your comments :)
Hanjun
^ permalink raw reply
* pinctrl: at91: drive strength control
From: Mark Roszko @ 2014-01-21 3:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121021925.GM9558@ns203013.ovh.net>
If I choose it at runtime what's the best way for me to detect the SoC
type to choose the correct register address at runtime? Only way I see
is adding new compatible types for the SAMA5s and the SAM9s with the
registers.
On Mon, Jan 20, 2014 at 9:19 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 20:07 Mon 20 Jan , Marek Roszko wrote:
>> Hello Nick and Jean-Christophe,
>>
>> I submit a idea/patch for comments. The current pintctrl driver is missing the ability to set the drive strength in the SAMA5D3s and a few of the SAM9s.
>> A little feature I myself need for the SAMA5D3s because somebody let an analog engineer do the hardware design.
>>
>> Issues with this patch I need help with:
>> 1. Atmel for some reason shifted the PIO_DRIVER1 and PIO_DRIVER2 registers by one register address between the SAM9 chips and the SAMA5D3s
>>
>> i.e. this is the SAM9s
>> #define PIO_DRIVER1_V1 0x114
>> #define PIO_DRIVER2_V1 0x118
>>
>> this is the SAMA5D3s
>> #define PIO_DRIVER1_V2 0x118
>> #define PIO_DRIVER2_V2 0x11C
>>
>>
>> 2. Atmel changed the meaning of value of "low", "medium" and "high" drive strengths between the two sets of chips as well.
>>
>> SAM9s do:
>> 00 = high
>> 01 = medium
>> 10 = low
>> 11 = reserved/undefined
>>
>> SAMA5D3s do:
>> 00 = low
>> 01 = low
>> 10 = medium
>> 11 = high
>>
>> 3. The SAM9G25, SAM9G35 have the PIO_DRIVER but the SAM9G45 does not have the PIO_DRIVER register as an example of
>> how oddly the behavior doesn't appear everywhere. This could also be because the datasheets for the G45 and G46 are
>> not updated to the new style while the others already have been?
>>
>> Side note: The SAMA5D3 datasheet says the default drive strength is "low" with register values 0x00000000,
>> in reality the default is "medium" with 0xAAAAAAAA as the default value.
>> Confirmed with the JLINK debugger and by Atmel support. Not an real issue, just a note when testing.
>
> check the coding style tab for indentatnion, 80 charrs per line max etc...
>>
>>
>> So the only simple way I see is to #ifdef the SOC type/chip.
>> There's probably better far better ways I don't know.
>>
>> ---
>> arch/arm/mach-at91/include/mach/at91_pio.h | 9 +++++
>> drivers/pinctrl/pinctrl-at91.c | 54 +++++++++++++++++++++++++++-
>> include/dt-bindings/pinctrl/at91.h | 13 +++++++
>> 3 files changed, 75 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-at91/include/mach/at91_pio.h b/arch/arm/mach-at91/include/mach/at91_pio.h
>> index 732b11c..28e8801 100644
>> --- a/arch/arm/mach-at91/include/mach/at91_pio.h
>> +++ b/arch/arm/mach-at91/include/mach/at91_pio.h
>> @@ -66,6 +66,15 @@
>> #define PIO_FRLHSR 0xd8 /* Fall/Rise - Low/High Status Register */
>> #define PIO_SCHMITT 0x100 /* Schmitt Trigger Register */
>>
>> +/*
>> + * SoC Specific PIO Address Offsets
>> + */
>> +#define PIO_DRIVER1_V1 0x114
>> +#define PIO_DRIVER2_V1 0x118
>> +
>> +#define PIO_DRIVER1_V2 0x118
>> +#define PIO_DRIVER2_V2 0x11C
>> +
>> #define ABCDSR_PERIPH_A 0x0
>> #define ABCDSR_PERIPH_B 0x1
>> #define ABCDSR_PERIPH_C 0x2
>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index a7549c4..ccf456e 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -62,10 +62,37 @@ static int gpio_banks;
>> #define DEGLITCH (1 << 2)
>> #define PULL_DOWN (1 << 3)
>> #define DIS_SCHMIT (1 << 4)
>> +#define SET_DRIVE_STRENGTH (1 << 5)
>> +#define DRIVE_STRENGTH_SHIFT 6
>> +#define DRIVE_STRENGTH (0x3 << DRIVE_STRENGTH_SHIFT)
>> #define DEBOUNCE (1 << 16)
>> #define DEBOUNCE_VAL_SHIFT 17
>> #define DEBOUNCE_VAL (0x3fff << DEBOUNCE_VAL_SHIFT)
>>
>> +#define DRIVE_STRENGTH_MASK 0x3
>> +
>> +#define NUM_PINS_PER_DRIVE_STRENGTH_REG 16
>> +
>> +#define TWO_BIT_PIN_TO_SHIFT(pin)\
>> + (2*((pin >= NUM_PINS_PER_DRIVE_STRENGTH_REG) ? \
>> + pin - NUM_PINS_PER_DRIVE_STRENGTH_REG : pin))
>
>> +
>> +#define TWO_BIT_PIN_TO_MASK(pin)\
>> + (DRIVE_STRENGTH_MASK << TWO_BIT_PIN_TO_SHIFT(pin))
>> +
>> +#if defined(CONFIG_SOC_SAMA5D3)
>> + #define PIO_DRIVER1 PIO_DRIVER1_V2 /* Drive Strength Register 1 */
>> + #define PIO_DRIVER2 PIO_DRIVER2_V2 /* Drive Strength Register 2 */
>> +#elif defined(CONFIG_SOC_AT91SAM9X5) || defined(SOC_AT91SAM9G35) || defined(SOC_AT91SAM9G25)
>> + #define PIO_DRIVER1 PIO_DRIVER1_V1 /* Drive Strength Register 1 */
>> + #define PIO_DRIVER2 PIO_DRIVER2_V1 /* Drive Strength Register 2 */
>> +#endif
> no ifdef this is choose at runtime
>> +
>> +#if defined(PIO_DRIVER1) && defined(PIO_DRIVER2)
>> + #define PIO_DRIVER(pin)\
>> + ((pin > NUM_PINS_PER_DRIVE_STRENGTH_REG-1) ? PIO_DRIVER2 : PIO_DRIVER1)
>> +#endif
> use inline function
>> +
>> /**
>> * struct at91_pmx_func - describes AT91 pinmux functions
>> * @name: the name of this specific function
>> @@ -148,6 +175,8 @@ struct at91_pinctrl_mux_ops {
>> void (*set_deglitch)(void __iomem *pio, unsigned mask, bool is_on);
>> bool (*get_debounce)(void __iomem *pio, unsigned pin, u32 *div);
>> void (*set_debounce)(void __iomem *pio, unsigned mask, bool is_on, u32 div);
>> + int (*get_drivestrength)(void __iomem *pio, unsigned pin);
>> + void (*set_drivestrength)(void __iomem *pio, unsigned pin, u32 strength);
>> bool (*get_pulldown)(void __iomem *pio, unsigned pin);
>> void (*set_pulldown)(void __iomem *pio, unsigned mask, bool is_on);
>> bool (*get_schmitt_trig)(void __iomem *pio, unsigned pin);
>> @@ -462,6 +491,20 @@ static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is
>> __raw_writel(mask, pio + (is_on ? PIO_PPDER : PIO_PPDDR));
>> }
>>
>> +static int at91_mux_pio3_get_drivestrength(void __iomem *pio, unsigned pin)
>> +{
>> + return (__raw_readl(pio + PIO_DRIVER(pin)) >> TWO_BIT_PIN_TO_SHIFT(pin)) & DRIVE_STRENGTH_MASK;
>> +}
>> +
>> +static void at91_mux_pio3_set_drivestrength(void __iomem *pio, unsigned pin, u32 strength)
>> +{
>> + unsigned tmp = __raw_readl(pio + PIO_DRIVER(pin));
>> + tmp &= ~TWO_BIT_PIN_TO_MASK(pin);
>> + tmp |= strength << TWO_BIT_PIN_TO_SHIFT(pin);
>> +
>> + __raw_writel(tmp, pio + PIO_DRIVER(pin));
>> +}
>> +
>> static void at91_mux_pio3_disable_schmitt_trig(void __iomem *pio, unsigned mask)
>> {
>> __raw_writel(__raw_readl(pio + PIO_SCHMITT) | mask, pio + PIO_SCHMITT);
>> @@ -491,6 +534,8 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
>> .set_deglitch = at91_mux_pio3_set_deglitch,
>> .get_debounce = at91_mux_pio3_get_debounce,
>> .set_debounce = at91_mux_pio3_set_debounce,
>> + .get_drivestrength = at91_mux_pio3_get_drivestrength,
>> + .set_drivestrength = at91_mux_pio3_set_drivestrength,
>> .get_pulldown = at91_mux_pio3_get_pulldown,
>> .set_pulldown = at91_mux_pio3_set_pulldown,
>> .get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
>> @@ -736,6 +781,8 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
>> *config |= DEGLITCH;
>> if (info->ops->get_debounce && info->ops->get_debounce(pio, pin, &div))
>> *config |= DEBOUNCE | (div << DEBOUNCE_VAL_SHIFT);
>> + if (info->ops->get_drivestrength)
>> + *config |= DRIVE_STRENGTH | (info->ops->get_drivestrength(pio, pin) << DRIVE_STRENGTH_SHIFT);
>> if (info->ops->get_pulldown && info->ops->get_pulldown(pio, pin))
>> *config |= PULL_DOWN;
>> if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
>> @@ -753,6 +800,7 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
>> void __iomem *pio;
>> int i;
>> unsigned long config;
>> + unsigned pin;
>>
>> for (i = 0; i < num_configs; i++) {
>> config = configs[i];
>> @@ -761,7 +809,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
>> "%s:%d, pin_id=%d, config=0x%lx",
>> __func__, __LINE__, pin_id, config);
>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>> - mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
>> + pin = pin_id % MAX_NB_GPIO_PER_BANK;
>> + mask = pin_to_mask(pin);
>>
>> if (config & PULL_UP && config & PULL_DOWN)
>> return -EINVAL;
>> @@ -773,6 +822,9 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
>> if (info->ops->set_debounce)
>> info->ops->set_debounce(pio, mask, config & DEBOUNCE,
>> (config & DEBOUNCE_VAL) >> DEBOUNCE_VAL_SHIFT);
>> + if (info->ops->set_drivestrength && config & SET_DRIVE_STRENGTH)
>> + info->ops->set_drivestrength(pio, pin,
>> + (config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
>> if (info->ops->set_pulldown)
>> info->ops->set_pulldown(pio, mask, config & PULL_DOWN);
>> if (info->ops->disable_schmitt_trig && config & DIS_SCHMIT)
>> diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
>> index 0fee6ff..78621a4 100644
>> --- a/include/dt-bindings/pinctrl/at91.h
>> +++ b/include/dt-bindings/pinctrl/at91.h
>> @@ -20,6 +20,19 @@
>>
>> #define AT91_PINCTRL_PULL_UP_DEGLITCH (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DEGLITCH)
>>
>> +#define AT91_PINCTRL_SET_DRIVE_STRENGTH (1 << 5)
>> +
>> +#if defined(CONFIG_SOC_SAMA5D3)
>> + #define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x2 << 6)
>> + #define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x1 << 6)
>> + #define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x0 << 6)
>> +#elif defined(CONFIG_SOC_AT91SAM9X5) || defined(SOC_AT91SAM9G35) || defined(SOC_AT91SAM9G25)
>> + #define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x1 << 6)
>> + #define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x2 << 6)
>> + #define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x3 << 6)
>> +#endif
>
> no only one binding we do not care of the IP details
>
> 0 means no drive strength specified
>> +
>> +
>> #define AT91_PIOA 0
>> #define AT91_PIOB 1
>> #define AT91_PIOC 2
>> --
>> 1.7.10.4
>>
>>
>>
>>
>> --
>> Mark
>>
>>
--
Mark
^ permalink raw reply
* [PATCH RFC 4/6] net: rfkill: gpio: add device tree support
From: Alexandre Courbot @ 2014-01-21 3:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdZOD4zeA8T5kbJ4c5NsnuzHCg1mw8rRMYNT9c4R-Qnc6A@mail.gmail.com>
On Sat, Jan 18, 2014 at 8:11 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jan 17, 2014 at 6:43 PM, Chen-Yu Tsai <wens@csie.org> wrote:
>> On Sat, Jan 18, 2014 at 12:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
>>>> +- NAME_shutdown-gpios : GPIO phandle to shutdown control
>>>> + (phandle must be the second)
>>>> +- NAME_reset-gpios : GPIO phandle to reset control
>>>> +
>>>> +NAME must match the rfkill-name property. NAME_shutdown-gpios or
>>>> +NAME_reset-gpios, or both, must be defined.
>>>> +
>>>
>>> I don't understand this part. Why do you include the name in the
>>> gpios property, rather than just hardcoding the property strings
>>> to "shutdown-gpios" and "reset-gpios"?
>>
>> This quirk is a result of how gpiod_get_index implements device tree
>> lookup.
>
> Why can't it just have a single property "gpios", where the first
> element is the reset GPIO and the second is the shutdown GPIO?
>
> rfkill-gpio does this:
>
> gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
> gpio = devm_gpiod_get_index(&pdev->dev, rfkill->shutdown_name, 1);
>
> The passed con ID name parameter is only there for the device
> tree case it seems. (ACPI ignores it.) So what about you just
> don't pass it at all and patch it to do like this instead:
>
> gpio = devm_gpiod_get_index(&pdev->dev, NULL, 0);
> gpio = devm_gpiod_get_index(&pdev->dev, NULL, 1);
>
> Heikki, are you OK with this change?
>
> I think this is actually necessary if the ACPI and DT unification
> pipe dream shall limp forward, we cannot have arguments passed
> that have a semantic effect on DT but not on ACPI... Drivers
> that are supposed to use both ACPI and DT will always
> have to pass NULL as con ID.
I agree that's how it should be be done with the current API if your
driver can obtain GPIOs from both ACPI and DT. This is a potential
issue, as drivers are not supposed to make assumptions about who is
going to be their GPIO provider. Let's say you started a driver with
only DT in mind, and used gpio_get(dev, con_id) to get your GPIOs. DT
bindings are thus of the form "con_id-gpio = <phandle>", and set in
stone. Then later, someone wants to use your driver with ACPI. How do
you handle that gracefully?
I'm starting to wonder, now that ACPI is a first-class GPIO provider,
whether we should not start to encourage the deprecation of the
"con_id-gpio = <phandle>" binding form in DT and only use a single
indexed GPIO property per device. The con_id parameter would then only
be used as a label, which would also have the nice side-effect that
all GPIOs used for a given function will be reported under the same
name no matter what the GPIO provider is.
>From an aesthetic point of view, I definitely prefer using con_id to
identify GPIOs instead of indexes, but I don't see how we can make it
play nice with ACPI. Thoughts?
Alex.
^ permalink raw reply
* More GPIO madness on iMX6 - and the crappy ARM port of Linux
From: Alexandre Courbot @ 2014-01-21 2:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdaRQRD85-0wUmip=j1SHzti6Nb69JEUXznYTkrxGvky=A@mail.gmail.com>
On Sat, Jan 18, 2014 at 7:43 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jan 17, 2014 at 9:53 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Fri, Jan 17, 2014 at 01:42:44PM -0700, Stephen Warren wrote:
>
>>> I believe you want gpio_get_value() to return either the driven or
>>> actual pin value where it can on the current HW, but just e.g. hard-code
>>> 0 on other HW. That would introduce a core feature that works some
>>> places but not others, and hence make drivers that relied on the feature
>>> less portable between HW with different actual features.
>>
>> I can buy that argument, but there's an issue which stands squarely in
>> its way, and that is open-drain GPIOs.
>>
>> These are modelled just as any other GPIO, mainly so that both
>> gpio_set_value(gpio, 1) and gpio_direction_input(gpio) both result in
>> the signal being high. The only combination which results in the
>> signal being driven low is outputting zero - and the state of the signal
>> can aways be read back.
>>
>> The problem here is that such gpios are implemented in things like the
>> I2C driver such that they're _always_ outputs, and gpio_set_value() is
>> used to pull the signal down. gpio_get_value() is used to read its
>> current state.
>>
>> So, if we say that gpio_get_value() is undefined, we force such
>> subsystems to always jump through the non-open-drain paths (using
>> gpio_direction_input() to set the line high and
>> gpio_direction_output(gpio, 0) to drive it low.)
>
> Incidentally that is what gpiolib is doing internally in
> gpiod_direction_output().
>
> You're absolutely right that it makes no sense to have open
> drain (or open source) unless the signal can be read back from
> the hardware.
>
> I'm thinking something like if the driver manages to obtain a
> GPIO with
>
> gpio_request_one(gpio, GPIOF_OPEN_DRAIN |
> GPIOF_OUT_INIT_HIGH);
>
> As the I2C core does, and then when that call succeeds, it can
> expect that whatever comes back from gpio_get_value() is
> always what is actually on the line. If the driver cannot determine
> this it should not have allowed that flag to succeed in the first
> place, so this might be something we want to enforce.
>
> There are two white spots on the map here:
>
> 1. Today this OPEN_DRAIN flag is not even passed down to
> the driver so how could it say anything about it :-( it's a pure gpiolib
> internal flag. We don't know if the hardware can actually even
> do open drain, we just assume it can.
>
> What it should really do - in the best of worlds - is to check if
> it can cross-reference the GPIO line to a pin in the pin control
> subsystem, and if that is possible, then ask the pin if it
> is supporting open drain and set it. It currently has no such
> cross-calls, it is just assumed that the configuration is consistent,
> and the actual pin is set up as open drain. But it would make
> sense to add more cross-calls here, since GPIO is accepting
> these flags (OPEN_DRAIN/OPEN_SOURCE).
This would definitely work in the case of pinctrl-backed GPIOs, but
would not cover all GPIO chips. If we want to cover all cases we
should give drivers a way to way to report or enforce this capability,
and make the pinctrl cross-reference one of its implementations where
it can be done.
>
> Like:
> int pinctrl_gpio_set_flags(unsigned gpio, unsigned long flags);
>
> Where the pinctrl subsystem would attempt to cross reference
> and set the flag, and the pin controller backend will then have
> the option to return an error code.
>
> We could atleast support that for the select pin controllers
> that use generic pin config. i.MX is another story, but I'm open
> to compromises.
>
> 2. In the new descriptor API this open drain setting would
> be set from the lookup table and be a property on the line,
> meaning this flag is not requested explicitly by the consumer,
> and the consumer needs to inspect the obtained descriptor
> to figure out if it is set to open drain.
>
> Alexandre: do you have plans for how to handle a dynamic
> consumer passing flags to its gpio request in the gpiod API?
Do you mean like passing OPEN_DRAIN or OPEN_SOURCE flags to
gpiod_get(), similarly to what is done for e.g. gpio_request_one()?
In the case of the gpiod API I would rather see these flags defined in
the GPIO mapping if possible. For platform data it is already possible
to specify open drain/open source, for DT this is trivial to add. ACPI
would be more of a problem here, but I'm not sure whether the problem
is relevant for ACPI GPIOs.
So the way I see it coming into shape would be something like:
1) GPIO drivers' request() function get an extra flags argument that
is passed by the GPIO core with the flags of the mapping. There we can
define all the range of properties that gpio_request_one() supported.
The driver's request() will fail it if cannot satisfy these
properties. That's where the pinctrl cross-reference would take place.
2) All properties accepted by gpio_request_one() can also be passed
through GPIO mappings. That is, probably platform_data and DT. I don't
know if we ever get to use open drain GPIOs provided by ACPI, if we do
then this might be a problem.
This does not address the initial problem, which is the uncertainty of
values returned by input GPIOs. For this either we enforce some more
strict rules, or we provide a function to allow drivers to check how
the value is reported, which is what you proposed below:
> I noticed that API missing now, there is exactly one user in the
> entire kernel, in drivers/i2c/i2c-core.c but a very important one.
>
> I guess to switch the I2C core over to descriptors I could
> think of an API like this:
>
> int gpiod_get_flags(const struct gpio_desc *desc);
>
> If the OPEN_DRAIN flag is set on that descriptor we should
> always be able to read the input. But as this is not really what the
> I2C core wants to know (it really would prefer not to bother with
> such GPIO flag details) so is it better if we add a special call to
> figure out if the input can be read? Like:
>
> bool gpiod_input_always_valid(const struct gpio_desc *desc);
>
> And leave it up to the core to look at flags, driver characteristics
> etc and determine whether the input can be trusted?
I am personally a little bit scared by the number of exported
functions in the GPIO framework. It is a pretty large number for
something that is supposed to be simple, so I'd like to avoid adding
more. :) How about the following:
1) GPIOs configured as output without the open drain or open source
flag either return -EINVAL on gpiod_get_value(), or a cached value
tracked by gpiolib for consistency (probably the latter).
2) GPIOs configured as open drain or open source report the actual
value read on the pin, like i2c-core needs. This requires that, for
each GPIO that can be set open drain or open source,
gpiod_input_always_valid() == true.
This is probably naive and needs to be refined, but I wonder if we
could not come with a relatively simple behavior that would lift
ambiguities without complexifying the API. Whatever we come with, we
will also need to think about how we can make the change without
breaking too many users.
Alex.
^ permalink raw reply
* [PATCH v3 1/2] i2c: qup: Add device tree bindings information
From: Stephen Boyd @ 2014-01-21 2:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqJBEu8iLzE8-dXniZFytr6cQsprBYiYnJeXS2-80iy1Mw@mail.gmail.com>
On 01/20, Rob Herring wrote:
> On Fri, Jan 17, 2014 at 5:03 PM, Bjorn Andersson
> <bjorn.andersson@sonymobile.com> wrote:
>
> > .../devicetree/bindings/i2c/qcom,i2c-qup.txt | 41 ++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> > new file mode 100644
> > index 0000000..a99711b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> > @@ -0,0 +1,41 @@
> > +Qualcomm Universal Peripheral (QUP) I2C controller
> > +
> > +Required properties:
> > + - compatible: Should be "qcom,i2c-qup".
>
> Seems a bit generic. All versions of the IP are exactly the same?
> "qcom,<chip>-i2c-qup" would be better.
>
There are different versions of the IP in different SoCs. The
versions for platforms supported upstream are:
1.1.1 (MSM8660, MSM8960)
2.2.1 (MSM8974v2)
so how about one of "qcom,i2c-qup-v1.1.1" or
"qcom,i2c-qup-v2.2.1" should be specified as more specific
compatible strings in addition to "qcom,i2c-qup"?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH v4 0/7] mtd: spi-nor: add a new framework for SPI NOR
From: Huang Shijie @ 2014-01-21 2:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD6G_RTwMJ5DgkCB7wcYw+2p---DqU4KCVJ3CUnHFfCJ0ui_Jw@mail.gmail.com>
On Sun, Jan 19, 2014 at 03:39:34PM +0530, Jagan Teki wrote:
> > Just think about that how do you code your driver for SPI NOR _without_ this
> > framework:
> > do your driver need to call the m25p80.c? if you do call the m25p80.c, you
> > actually write your so-called "two drivers" :)
>
> I'm still not convinced here, sorry!
> Let me explain more with example, Just take your (q)spi controller as an example
> B_FSL1 designed with - serial flash
> B_FSL2 designed with - touch screen
>
> With this design If we follow current we just need one controller
> driver (drivers/spi/spi-fsl.c)
> to serve touch screen and serial flash from upper layers.
>
> MTD core Input core
> ------------- -------------------------------
> m25p80.c spi touchscreen driver (drivers/input/touchscreen)
> ----------------------------------------------------
> SPI core
> ----------------------------------------------------
> controller driver (drivers/spi/spi-fsl.)
> ----------------------------------------------------
>
> But with your new SPI-NOR
>
> MTD core
> ------------
> SPI-NOR Input core
> ----------- ------------------
> fsl-quadspi.c spi touchscreen driver (drivers/input/touchscreen)
> ---------------- ------------------------------
> SPI Core
> -----------------------------
> controller driver (drivers/spi/spi-fsl.c)
> -----------------------------------------
>
There is no relationship with the fsl-quadspi.c.
It should be like this:
MTD core
------------
SPI-NOR framework Input core
----------- ------------------
m25p80.c spi touchscreen driver (drivers/input/touchscreen)
---------------- ------------------------------
SPI Core SPI Core
----------------------- -----------------------------
drivers/spi/spi-fsl.c controller driver (drivers/spi/spi-fsl.c)
-------------------------- -----------------------------------------
> Here see we have two separate drivers for the same controller as it offers two
> functionalities. and also it never talk to Linux SPI subsystem which is not a
> good for me as intern hw is SPI-based it must talk to subsystem to make use of
> Linux SPI API's.
>
> And also you mentioned SPI-NOR controller is not a SPI controller what does that
> means because it's a NOR flash which is of SPI protocol based.
The SPI controller (or spi bus controller) only handles the _byte_ streams;
while the spi-nor controller, such as fsl-quadspi.c, can has to __known__
the exact SPI NOR command to find the proper LUT.
Please read the thread i ever posted in the previous email.
>
> I understand that your framework doing good things in matured manner, but
> seems like it's confuse to me to follow common standards - Just my
> opinion, may be I'm incorrect.
>
> One more thing question - I have 1-wire spi controller which I was
> used for flash in
> one board and keyboard in another board, so if I follow your framework
> I need to write
> a driver in drivers/mtd/spi-nor/foo-spi.c and one more in
> drivers/spi/spi-foo.c - correct?
Please read the device tree node of your controller, find the code like
...................................................
flash: m25p80 at 0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "st,m25p32";
spi-max-frequency = <20000000>;
reg = <0>;
};
...................................................
if you call the m25p80, you do not need to change anything.
thanks
Huang Shijie
^ permalink raw reply
* [PATCH v3 2/2] i2c: New bus driver for the QUP I2C controller
From: Stephen Boyd @ 2014-01-21 2:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389999819-10648-3-git-send-email-bjorn.andersson@sonymobile.com>
On 01/17, Bjorn Andersson wrote:
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> new file mode 100644
> index 0000000..2e0020e
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -0,0 +1,894 @@
> +/* Copyright (c) 2009-2013, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only 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/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +/* QUP Registers */
> +#define QUP_CONFIG 0x000
> +#define QUP_STATE 0x004
> +#define QUP_IO_MODE 0x008
> +#define QUP_SW_RESET 0x00c
> +#define QUP_OPERATIONAL 0x018
> +#define QUP_ERROR_FLAGS 0x01c
> +#define QUP_ERROR_FLAGS_EN 0x020
> +#define QUP_HW_VERSION 0x030
> +#define QUP_MX_OUTPUT_CNT 0x100
> +#define QUP_OUT_FIFO_BASE 0x110
> +#define QUP_MX_WRITE_CNT 0x150
> +#define QUP_MX_INPUT_CNT 0x200
> +#define QUP_MX_READ_CNT 0x208
> +#define QUP_IN_FIFO_BASE 0x218
> +#define QUP_I2C_CLK_CTL 0x400
> +#define QUP_I2C_STATUS 0x404
> +
> +/* QUP States and reset values */
> +#define QUP_RESET_STATE 0
> +#define QUP_RUN_STATE 1
> +#define QUP_PAUSE_STATE 3
> +#define QUP_STATE_MASK 3
> +
> +#define QUP_STATE_VALID BIT(2)
> +#define QUP_I2C_MAST_GEN BIT(4)
> +
> +#define QUP_OPERATIONAL_RESET 0x000ff0
> +#define QUP_I2C_STATUS_RESET 0xfffffc
> +
> +/* QUP OPERATIONAL FLAGS */
> +#define QUP_OUT_SVC_FLAG BIT(8)
> +#define QUP_IN_SVC_FLAG BIT(9)
> +#define QUP_MX_INPUT_DONE BIT(11)
> +
> +/* I2C mini core related values */
> +#define I2C_MINI_CORE (2 << 8)
> +#define I2C_N_VAL 15
> +/* Most significant word offset in FIFO port */
> +#define QUP_MSW_SHIFT (I2C_N_VAL + 1)
> +#define QUP_CLOCK_AUTO_GATE BIT(13)
> +
> +/* Packing/Unpacking words in FIFOs, and IO modes */
> +#define QUP_UNPACK_EN BIT(14)
> +#define QUP_PACK_EN BIT(15)
> +#define QUP_OUTPUT_BLK_MODE BIT(10)
> +#define QUP_INPUT_BLK_MODE BIT(12)
> +
> +#define QUP_REPACK_EN (QUP_UNPACK_EN | QUP_PACK_EN)
> +
> +#define QUP_OUTPUT_BLOCK_SIZE(x)(((x) & (0x03 << 0)) >> 0)
> +#define QUP_OUTPUT_FIFO_SIZE(x) (((x) & (0x07 << 2)) >> 2)
> +#define QUP_INPUT_BLOCK_SIZE(x) (((x) & (0x03 << 5)) >> 5)
> +#define QUP_INPUT_FIFO_SIZE(x) (((x) & (0x07 << 7)) >> 7)
> +
> +/* QUP tags */
> +#define QUP_OUT_NOP (0 << 8)
> +#define QUP_OUT_START (1 << 8)
> +#define QUP_OUT_DATA (2 << 8)
> +#define QUP_OUT_STOP (3 << 8)
> +#define QUP_OUT_REC (4 << 8)
> +#define QUP_IN_DATA (5 << 8)
> +#define QUP_IN_STOP (6 << 8)
> +#define QUP_IN_NACK (7 << 8)
> +
> +/* Status, Error flags */
> +#define I2C_STATUS_WR_BUFFER_FULL BIT(0)
> +#define I2C_STATUS_BUS_ACTIVE BIT(8)
> +#define I2C_STATUS_BUS_MASTER BIT(9)
> +#define I2C_STATUS_ERROR_MASK 0x38000fc
> +#define QUP_I2C_NACK_FLAG BIT(3)
> +#define QUP_IN_NOT_EMPTY BIT(5)
> +#define QUP_STATUS_ERROR_FLAGS 0x7c
> +
> +/* Master bus_err clock states */
> +#define I2C_CLK_RESET_BUSIDLE_STATE 0
> +#define I2C_CLK_FORCED_LOW_STATE 5
> +
> +#define QUP_MAX_CLK_STATE_RETRIES 300
> +#define QUP_MAX_QUP_STATE_RETRIES 100
> +#define I2C_STATUS_CLK_STATE 13
> +#define QUP_OUT_FIFO_NOT_EMPTY 0x10
> +#define QUP_READ_LIMIT 256
> +
> +struct qup_i2c_dev {
> + struct device *dev;
> + void __iomem *base;
> + int irq;
> + struct clk *clk;
> + struct clk *pclk;
> + struct i2c_adapter adap;
> +
> + int clk_ctl;
> + int one_bit_t;
> + int out_fifo_sz;
> + int in_fifo_sz;
> + int out_blk_sz;
> + int in_blk_sz;
> + unsigned long xfer_time;
> + unsigned long wait_idle;
> +
> + struct i2c_msg *msg;
> + /* Current posion in user message buffer */
s/posion/position/
> + int pos;
> + /* Keep number of bytes left to be transmitted */
> + int cnt;
> + /* I2C protocol errors */
> + u32 bus_err;
> + /* QUP core errors */
> + u32 qup_err;
> + /*
> + * Maximum bytes that could be send (per iteration). Could be
> + * equal of fifo size or block size (in block mode)
> + */
> + int chunk_sz;
> + struct completion xfer;
> +};
[...]
> +
> +static int
> +qup_i2c_poll_state(struct qup_i2c_dev *qup, u32 req_state, bool only_valid)
> +{
> + int retries = 0;
> + u32 state;
> +
> + do {
> + state = readl(qup->base + QUP_STATE);
> +
> + /*
> + * If only valid bit needs to be checked, requested state is
> + * 'don't care'
> + */
> + if (state & QUP_STATE_VALID) {
> + if (only_valid)
> + return 0;
> + if ((req_state & QUP_I2C_MAST_GEN)
> + && (state & QUP_I2C_MAST_GEN))
> + return 0;
> + if ((state & QUP_STATE_MASK) == req_state)
> + return 0;
> + }
> +
> + udelay(1);
> + } while (retries++ != QUP_MAX_QUP_STATE_RETRIES);
> +
> + return -ETIMEDOUT;
> +}
> +
This is what I was suggesting. Don't know if it's any clearer,
but it saves us a whopping 3 lines!
---8<----
drivers/i2c/busses/i2c-qup.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 2e0020e829ae..431de13f6281 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -47,7 +47,7 @@
#define QUP_STATE_MASK 3
#define QUP_STATE_VALID BIT(2)
-#define QUP_I2C_MAST_GEN BIT(4)
+#define QUP_I2C_MAST_GEN (QUP_STATE_VALID | BIT(4))
#define QUP_OPERATIONAL_RESET 0x000ff0
#define QUP_I2C_STATUS_RESET 0xfffffc
@@ -190,43 +190,40 @@ done:
return IRQ_HANDLED;
}
-static int
-qup_i2c_poll_state(struct qup_i2c_dev *qup, u32 req_state, bool only_valid)
+static int __qup_i2c_poll_state(struct qup_i2c_dev *qup, u32 mask, u32 value)
{
int retries = 0;
u32 state;
do {
state = readl(qup->base + QUP_STATE);
-
- /*
- * If only valid bit needs to be checked, requested state is
- * 'don't care'
- */
- if (state & QUP_STATE_VALID) {
- if (only_valid)
- return 0;
- if ((req_state & QUP_I2C_MAST_GEN)
- && (state & QUP_I2C_MAST_GEN))
- return 0;
- if ((state & QUP_STATE_MASK) == req_state)
- return 0;
- }
-
+ if ((state & mask) == value)
+ return 0;
udelay(1);
} while (retries++ != QUP_MAX_QUP_STATE_RETRIES);
return -ETIMEDOUT;
}
+static int qup_i2c_poll_state_bit(struct qup_i2c_dev *qup, u32 mask)
+{
+ return __qup_i2c_poll_state(qup, mask, mask);
+}
+
+static int qup_i2c_poll_state(struct qup_i2c_dev *qup, u32 state)
+{
+ return __qup_i2c_poll_state(qup, QUP_STATE_VALID | QUP_STATE_MASK,
+ QUP_STATE_VALID | state);
+}
+
static int qup_i2c_change_state(struct qup_i2c_dev *qup, u32 state)
{
- if (qup_i2c_poll_state(qup, 0, true) != 0)
+ if (qup_i2c_poll_state_bit(qup, QUP_STATE_VALID) != 0)
return -EIO;
writel(state, qup->base + QUP_STATE);
- if (qup_i2c_poll_state(qup, state, false) != 0)
+ if (qup_i2c_poll_state(qup, state) != 0)
return -EIO;
return 0;
}
@@ -616,7 +613,7 @@ qup_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
goto out;
writel(1, qup->base + QUP_SW_RESET);
- ret = qup_i2c_poll_state(qup, QUP_RESET_STATE, false);
+ ret = qup_i2c_poll_state(qup, QUP_RESET_STATE);
if (ret) {
dev_err(qup->dev, "cannot goto reset state\n");
goto out;
@@ -633,7 +630,7 @@ qup_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
writel(0, qup->base + QUP_I2C_CLK_CTL);
writel(QUP_I2C_STATUS_RESET, qup->base + QUP_I2C_STATUS);
- if (qup_i2c_poll_state(qup, QUP_I2C_MAST_GEN, false) != 0) {
+ if (qup_i2c_poll_state_bit(qup, QUP_I2C_MAST_GEN) != 0) {
ret = -EIO;
goto out;
}
@@ -730,7 +727,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
* so we reset the core before registering for interrupts.
*/
writel(1, qup->base + QUP_SW_RESET);
- ret = qup_i2c_poll_state(qup, 0, true);
+ ret = qup_i2c_poll_state(qup, QUP_STATE_VALID);
if (ret)
goto fail;
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* pinctrl: at91: drive strength control
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-21 2:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390266470.26159.1.camel@bb>
On 20:07 Mon 20 Jan , Marek Roszko wrote:
> Hello Nick and Jean-Christophe,
>
> I submit a idea/patch for comments. The current pintctrl driver is missing the ability to set the drive strength in the SAMA5D3s and a few of the SAM9s.
> A little feature I myself need for the SAMA5D3s because somebody let an analog engineer do the hardware design.
>
> Issues with this patch I need help with:
> 1. Atmel for some reason shifted the PIO_DRIVER1 and PIO_DRIVER2 registers by one register address between the SAM9 chips and the SAMA5D3s
>
> i.e. this is the SAM9s
> #define PIO_DRIVER1_V1 0x114
> #define PIO_DRIVER2_V1 0x118
>
> this is the SAMA5D3s
> #define PIO_DRIVER1_V2 0x118
> #define PIO_DRIVER2_V2 0x11C
>
>
> 2. Atmel changed the meaning of value of "low", "medium" and "high" drive strengths between the two sets of chips as well.
>
> SAM9s do:
> 00 = high
> 01 = medium
> 10 = low
> 11 = reserved/undefined
>
> SAMA5D3s do:
> 00 = low
> 01 = low
> 10 = medium
> 11 = high
>
> 3. The SAM9G25, SAM9G35 have the PIO_DRIVER but the SAM9G45 does not have the PIO_DRIVER register as an example of
> how oddly the behavior doesn't appear everywhere. This could also be because the datasheets for the G45 and G46 are
> not updated to the new style while the others already have been?
>
> Side note: The SAMA5D3 datasheet says the default drive strength is "low" with register values 0x00000000,
> in reality the default is "medium" with 0xAAAAAAAA as the default value.
> Confirmed with the JLINK debugger and by Atmel support. Not an real issue, just a note when testing.
check the coding style tab for indentatnion, 80 charrs per line max etc...
>
>
> So the only simple way I see is to #ifdef the SOC type/chip.
> There's probably better far better ways I don't know.
>
> ---
> arch/arm/mach-at91/include/mach/at91_pio.h | 9 +++++
> drivers/pinctrl/pinctrl-at91.c | 54 +++++++++++++++++++++++++++-
> include/dt-bindings/pinctrl/at91.h | 13 +++++++
> 3 files changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/include/mach/at91_pio.h b/arch/arm/mach-at91/include/mach/at91_pio.h
> index 732b11c..28e8801 100644
> --- a/arch/arm/mach-at91/include/mach/at91_pio.h
> +++ b/arch/arm/mach-at91/include/mach/at91_pio.h
> @@ -66,6 +66,15 @@
> #define PIO_FRLHSR 0xd8 /* Fall/Rise - Low/High Status Register */
> #define PIO_SCHMITT 0x100 /* Schmitt Trigger Register */
>
> +/*
> + * SoC Specific PIO Address Offsets
> + */
> +#define PIO_DRIVER1_V1 0x114
> +#define PIO_DRIVER2_V1 0x118
> +
> +#define PIO_DRIVER1_V2 0x118
> +#define PIO_DRIVER2_V2 0x11C
> +
> #define ABCDSR_PERIPH_A 0x0
> #define ABCDSR_PERIPH_B 0x1
> #define ABCDSR_PERIPH_C 0x2
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a7549c4..ccf456e 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -62,10 +62,37 @@ static int gpio_banks;
> #define DEGLITCH (1 << 2)
> #define PULL_DOWN (1 << 3)
> #define DIS_SCHMIT (1 << 4)
> +#define SET_DRIVE_STRENGTH (1 << 5)
> +#define DRIVE_STRENGTH_SHIFT 6
> +#define DRIVE_STRENGTH (0x3 << DRIVE_STRENGTH_SHIFT)
> #define DEBOUNCE (1 << 16)
> #define DEBOUNCE_VAL_SHIFT 17
> #define DEBOUNCE_VAL (0x3fff << DEBOUNCE_VAL_SHIFT)
>
> +#define DRIVE_STRENGTH_MASK 0x3
> +
> +#define NUM_PINS_PER_DRIVE_STRENGTH_REG 16
> +
> +#define TWO_BIT_PIN_TO_SHIFT(pin)\
> + (2*((pin >= NUM_PINS_PER_DRIVE_STRENGTH_REG) ? \
> + pin - NUM_PINS_PER_DRIVE_STRENGTH_REG : pin))
> +
> +#define TWO_BIT_PIN_TO_MASK(pin)\
> + (DRIVE_STRENGTH_MASK << TWO_BIT_PIN_TO_SHIFT(pin))
> +
> +#if defined(CONFIG_SOC_SAMA5D3)
> + #define PIO_DRIVER1 PIO_DRIVER1_V2 /* Drive Strength Register 1 */
> + #define PIO_DRIVER2 PIO_DRIVER2_V2 /* Drive Strength Register 2 */
> +#elif defined(CONFIG_SOC_AT91SAM9X5) || defined(SOC_AT91SAM9G35) || defined(SOC_AT91SAM9G25)
> + #define PIO_DRIVER1 PIO_DRIVER1_V1 /* Drive Strength Register 1 */
> + #define PIO_DRIVER2 PIO_DRIVER2_V1 /* Drive Strength Register 2 */
> +#endif
no ifdef this is choose at runtime
> +
> +#if defined(PIO_DRIVER1) && defined(PIO_DRIVER2)
> + #define PIO_DRIVER(pin)\
> + ((pin > NUM_PINS_PER_DRIVE_STRENGTH_REG-1) ? PIO_DRIVER2 : PIO_DRIVER1)
> +#endif
use inline function
> +
> /**
> * struct at91_pmx_func - describes AT91 pinmux functions
> * @name: the name of this specific function
> @@ -148,6 +175,8 @@ struct at91_pinctrl_mux_ops {
> void (*set_deglitch)(void __iomem *pio, unsigned mask, bool is_on);
> bool (*get_debounce)(void __iomem *pio, unsigned pin, u32 *div);
> void (*set_debounce)(void __iomem *pio, unsigned mask, bool is_on, u32 div);
> + int (*get_drivestrength)(void __iomem *pio, unsigned pin);
> + void (*set_drivestrength)(void __iomem *pio, unsigned pin, u32 strength);
> bool (*get_pulldown)(void __iomem *pio, unsigned pin);
> void (*set_pulldown)(void __iomem *pio, unsigned mask, bool is_on);
> bool (*get_schmitt_trig)(void __iomem *pio, unsigned pin);
> @@ -462,6 +491,20 @@ static void at91_mux_pio3_set_pulldown(void __iomem *pio, unsigned mask, bool is
> __raw_writel(mask, pio + (is_on ? PIO_PPDER : PIO_PPDDR));
> }
>
> +static int at91_mux_pio3_get_drivestrength(void __iomem *pio, unsigned pin)
> +{
> + return (__raw_readl(pio + PIO_DRIVER(pin)) >> TWO_BIT_PIN_TO_SHIFT(pin)) & DRIVE_STRENGTH_MASK;
> +}
> +
> +static void at91_mux_pio3_set_drivestrength(void __iomem *pio, unsigned pin, u32 strength)
> +{
> + unsigned tmp = __raw_readl(pio + PIO_DRIVER(pin));
> + tmp &= ~TWO_BIT_PIN_TO_MASK(pin);
> + tmp |= strength << TWO_BIT_PIN_TO_SHIFT(pin);
> +
> + __raw_writel(tmp, pio + PIO_DRIVER(pin));
> +}
> +
> static void at91_mux_pio3_disable_schmitt_trig(void __iomem *pio, unsigned mask)
> {
> __raw_writel(__raw_readl(pio + PIO_SCHMITT) | mask, pio + PIO_SCHMITT);
> @@ -491,6 +534,8 @@ static struct at91_pinctrl_mux_ops at91sam9x5_ops = {
> .set_deglitch = at91_mux_pio3_set_deglitch,
> .get_debounce = at91_mux_pio3_get_debounce,
> .set_debounce = at91_mux_pio3_set_debounce,
> + .get_drivestrength = at91_mux_pio3_get_drivestrength,
> + .set_drivestrength = at91_mux_pio3_set_drivestrength,
> .get_pulldown = at91_mux_pio3_get_pulldown,
> .set_pulldown = at91_mux_pio3_set_pulldown,
> .get_schmitt_trig = at91_mux_pio3_get_schmitt_trig,
> @@ -736,6 +781,8 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config |= DEGLITCH;
> if (info->ops->get_debounce && info->ops->get_debounce(pio, pin, &div))
> *config |= DEBOUNCE | (div << DEBOUNCE_VAL_SHIFT);
> + if (info->ops->get_drivestrength)
> + *config |= DRIVE_STRENGTH | (info->ops->get_drivestrength(pio, pin) << DRIVE_STRENGTH_SHIFT);
> if (info->ops->get_pulldown && info->ops->get_pulldown(pio, pin))
> *config |= PULL_DOWN;
> if (info->ops->get_schmitt_trig && info->ops->get_schmitt_trig(pio, pin))
> @@ -753,6 +800,7 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> void __iomem *pio;
> int i;
> unsigned long config;
> + unsigned pin;
>
> for (i = 0; i < num_configs; i++) {
> config = configs[i];
> @@ -761,7 +809,8 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> - mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
> + pin = pin_id % MAX_NB_GPIO_PER_BANK;
> + mask = pin_to_mask(pin);
>
> if (config & PULL_UP && config & PULL_DOWN)
> return -EINVAL;
> @@ -773,6 +822,9 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> if (info->ops->set_debounce)
> info->ops->set_debounce(pio, mask, config & DEBOUNCE,
> (config & DEBOUNCE_VAL) >> DEBOUNCE_VAL_SHIFT);
> + if (info->ops->set_drivestrength && config & SET_DRIVE_STRENGTH)
> + info->ops->set_drivestrength(pio, pin,
> + (config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
> if (info->ops->set_pulldown)
> info->ops->set_pulldown(pio, mask, config & PULL_DOWN);
> if (info->ops->disable_schmitt_trig && config & DIS_SCHMIT)
> diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
> index 0fee6ff..78621a4 100644
> --- a/include/dt-bindings/pinctrl/at91.h
> +++ b/include/dt-bindings/pinctrl/at91.h
> @@ -20,6 +20,19 @@
>
> #define AT91_PINCTRL_PULL_UP_DEGLITCH (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DEGLITCH)
>
> +#define AT91_PINCTRL_SET_DRIVE_STRENGTH (1 << 5)
> +
> +#if defined(CONFIG_SOC_SAMA5D3)
> + #define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x2 << 6)
> + #define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x1 << 6)
> + #define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x0 << 6)
> +#elif defined(CONFIG_SOC_AT91SAM9X5) || defined(SOC_AT91SAM9G35) || defined(SOC_AT91SAM9G25)
> + #define AT91_PINCTRL_DRIVE_STRENGTH_LOW (0x1 << 6)
> + #define AT91_PINCTRL_DRIVE_STRENGTH_MED (0x2 << 6)
> + #define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x3 << 6)
> +#endif
no only one binding we do not care of the IP details
0 means no drive strength specified
> +
> +
> #define AT91_PIOA 0
> #define AT91_PIOB 1
> #define AT91_PIOC 2
> --
> 1.7.10.4
>
>
>
>
> --
> Mark
>
>
^ permalink raw reply
* [PATCH] ARM: asm: __und_usr_thumb need byteswap instructions in BE case
From: Stephen Boyd @ 2014-01-21 1:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140114090518.GA2222@e103592.cambridge.arm.com>
On 01/14/14 01:05, Dave Martin wrote:
> On Mon, Jan 13, 2014 at 02:30:51PM -0800, Victor Kamensky wrote:
>> __und_usr_thumb function deals with thumb2 opcodes. In case of BE
>> image, it needs to byteswap half word thumb2 encoded instructions
>> before further processing them.
>>
>> Without this fix BE image user-land thread executing first VFP
>> instruction encoded in thumb2 fails with SIGILL, because kernel
>> does not recognize instruction and does not enable VFP.
>>
>> Reported-by: Corey Melton <comelton@cisco.com>
>> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> Good spot. This looks like the correct fix to me.
>
> Acked-by: Dave Martin <Dave.Martin@arm.com>
>
Can you please send this to the patch tracker? Feel free to add
Tested-by: Stephen Boyd <sboyd@codeaurora.org>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ 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