* [PATCH 4/4] arm64: documentation: document tagged pointer stack constraints
From: Dave P Martin @ 2017-04-21 17:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492712234-4950-5-git-send-email-kristina.martsenko@arm.com>
On Thu, Apr 20, 2017 at 07:17:14PM +0100, Kristina Martsenko wrote:
> Some kernel features don't currently work if a task puts a non-zero
> address tag in its stack pointer, frame pointer, or frame record entries
> (FP, LR).
>
> For example, with a tagged stack pointer, the kernel can't deliver
> signals to the process, and the task is killed instead. As another
> example, with a tagged frame pointer or frame records, perf fails to
> generate call graphs or resolve symbols.
>
> For now, just document these limitations, instead of finding and fixing
> everything that doesn't work, as it's not known if anyone needs to use
> tags in these places anyway.
>
> In addition, as requested by Dave Martin, generalize the limitations
> into a general kernel address tag policy, and refactor
> tagged-pointers.txt to include it.
>
> Cc: Dave Martin <dave.martin@arm.com>
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
Looks OK to me. It's not feasible to describe all possible scenarios
here, but this makes the kernel's intentions clearer.
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
> ---
> Documentation/arm64/tagged-pointers.txt | 62 +++++++++++++++++++++++++--------
> 1 file changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/arm64/tagged-pointers.txt b/Documentation/arm64/tagged-pointers.txt
> index d9995f1f51b3..a25a99e82bb1 100644
> --- a/Documentation/arm64/tagged-pointers.txt
> +++ b/Documentation/arm64/tagged-pointers.txt
> @@ -11,24 +11,56 @@ in AArch64 Linux.
> The kernel configures the translation tables so that translations made
> via TTBR0 (i.e. userspace mappings) have the top byte (bits 63:56) of
> the virtual address ignored by the translation hardware. This frees up
> -this byte for application use, with the following caveats:
> +this byte for application use.
>
> - (1) The kernel requires that all user addresses passed to EL1
> - are tagged with tag 0x00. This means that any syscall
> - parameters containing user virtual addresses *must* have
> - their top byte cleared before trapping to the kernel.
>
> - (2) Non-zero tags are not preserved when delivering signals.
> - This means that signal handlers in applications making use
> - of tags cannot rely on the tag information for user virtual
> - addresses being maintained for fields inside siginfo_t.
> - One exception to this rule is for signals raised in response
> - to watchpoint debug exceptions, where the tag information
> - will be preserved.
> +Passing tagged addresses to the kernel
> +--------------------------------------
>
> - (3) Special care should be taken when using tagged pointers,
> - since it is likely that C compilers will not hazard two
> - virtual addresses differing only in the upper byte.
> +All interpretation of userspace memory addresses by the kernel assumes
> +an address tag of 0x00.
> +
> +This includes, but is not limited to, addresses found in:
> +
> + - pointer arguments to system calls, including pointers in structures
> + passed to system calls,
> +
> + - the stack pointer (sp), e.g. when interpreting it to deliver a
> + signal,
> +
> + - the frame pointer (x29) and frame records, e.g. when interpreting
> + them to generate a backtrace or call graph.
> +
> +Using non-zero address tags in any of these locations may result in an
> +error code being returned, a (fatal) signal being raised, or other modes
> +of failure.
> +
> +For these reasons, passing non-zero address tags to the kernel via
> +system calls is forbidden, and using a non-zero address tag for sp is
> +strongly discouraged.
> +
> +Programs maintaining a frame pointer and frame records that use non-zero
> +address tags may suffer impaired or inaccurate debug and profiling
> +visibility.
> +
> +
> +Preserving tags
> +---------------
> +
> +Non-zero tags are not preserved when delivering signals. This means that
> +signal handlers in applications making use of tags cannot rely on the
> +tag information for user virtual addresses being maintained for fields
> +inside siginfo_t. One exception to this rule is for signals raised in
> +response to watchpoint debug exceptions, where the tag information will
> +be preserved.
>
> The architecture prevents the use of a tagged PC, so the upper byte will
> be set to a sign-extension of bit 55 on exception return.
> +
> +
> +Other considerations
> +--------------------
> +
> +Special care should be taken when using tagged pointers, since it is
> +likely that C compilers will not hazard two virtual addresses differing
> +only in the upper byte.
> --
> 2.1.4
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply
* [PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling
From: Bjorn Helgaas @ 2017-04-21 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421170414.GA2429@localhost>
On Fri, Apr 21, 2017 at 05:05:41PM +0000, Jayachandran C wrote:
> On Fri, Apr 21, 2017 at 10:48:15AM -0500, Bjorn Helgaas wrote:
> > On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
> > <jnair@caviumnetworks.com> wrote:
> > > On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
> > >> Could you collect "lspci -vv" output from this system? I'd like to
> > >> archive that as background for this IOMMU issue and the ASPM tweaks I
> > >> suspect we'll have to do. I *wish* we had more information about that
> > >> VIA thing, because I suspect we could get rid of it if we had more
> > >> details.
> > >
> > > The full logs are slightly large, so I have kept them at:
> > > https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> > > The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
> > >
> > > The output is from 2 socket system, the cards are not on the first slot
> > > like the example above, so the bus and device numbers are different.
> >
> > Can somebody with this system collect the "lspci -xxxx" output as well?
> >
> > I'm making some lspci changes to handle the PCI-to-PCIe bridge
> > correctly, and I can use the "lspci -xxxx" output to create an lspci
> > test case.
>
> [Sorry was AFK for a few days]
>
> I have updated the above directory with the log. Also tested your next branch
> and it works fine on ThunderX2.
Thanks!
With regard to my lspci changes, they add "Slot-" here:
01:0a.0 PCI bridge: Broadcom Corporation Device 9039
...
- Capabilities: [40] Express (v2) PCI/PCI-X to PCI-Express Bridge, MSI 00
+ Capabilities: [40] Express (v2) PCI/PCI-X to PCI-Express Bridge (Slot-), MSI 00
for all your PCI-to-PCIe bridges. I assume the "Slot-" is correct, i.e.,
the link is not connected to a slot, right? This comes from the "Slot
Implemented" bit in the PCIe Capabilities Register.
I did notice that all the Root Port devices claim to *not* be connected to
slots, which doesn't seem right. For example,
12:00.0 PCI bridge: Broadcom Corporation Device 9084
Bus: primary=12, secondary=13, subordinate=14, sec-latency=0
Capabilities: [ac] Express (v2) Root Port (Slot-), MSI 00
13:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ Network Connection
It seems strange because the 12:00.0 Root Port looks like it probably
*does* lead to a slot where the NIC is plugged in. Or is that NIC really
soldered down?
But I assume there are *some* PCIe slots, so at some of those Root Ports
should advertise "Slot+" (which by itself does not imply hotplug support,
if that's the concern).
Bjorn
^ permalink raw reply
* [PATCH V15 04/11] efi: parse ARM processor error
From: Borislav Petkov @ 2017-04-21 17:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492556723-9189-5-git-send-email-tbaicar@codeaurora.org>
On Tue, Apr 18, 2017 at 05:05:16PM -0600, Tyler Baicar wrote:
> Add support for ARM Common Platform Error Record (CPER).
> UEFI 2.6 specification adds support for ARM specific
> processor error information to be reported as part of the
> CPER records. This provides more detail on for processor error logs.
>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Reviewed-by: James Morse <james.morse@arm.com>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/firmware/efi/cper.c | 135 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/cper.h | 54 ++++++++++++++++++
> 2 files changed, 189 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 46585f9..f959185 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -110,12 +110,15 @@ void cper_print_bits(const char *pfx, unsigned int bits,
> static const char * const proc_type_strs[] = {
> "IA32/X64",
> "IA64",
> + "ARM",
> };
>
> static const char * const proc_isa_strs[] = {
> "IA32",
> "IA64",
> "X64",
> + "ARM A32/T32",
> + "ARM A64",
> };
>
> static const char * const proc_error_type_strs[] = {
> @@ -184,6 +187,128 @@ static void cper_print_proc_generic(const char *pfx,
> printk("%s""IP: 0x%016llx\n", pfx, proc->ip);
> }
>
> +#if defined(CONFIG_ARM64) || defined(CONFIG_ARM)
> +static const char * const arm_reg_ctx_strs[] = {
> + "AArch32 general purpose registers",
> + "AArch32 EL1 context registers",
> + "AArch32 EL2 context registers",
> + "AArch32 secure context registers",
> + "AArch64 general purpose registers",
> + "AArch64 EL1 context registers",
> + "AArch64 EL2 context registers",
> + "AArch64 EL3 context registers",
> + "Misc. system register structure",
> +};
> +
> +static void cper_print_proc_arm(const char *pfx,
> + const struct cper_sec_proc_arm *proc)
> +{
> + int i, len, max_ctx_type;
> + struct cper_arm_err_info *err_info;
> + struct cper_arm_ctx_info *ctx_info;
> + char newpfx[64];
> +
> + printk("%ssection length: %d\n", pfx, proc->section_length);
We need to dump section length because?
> + printk("%sMIDR: 0x%016llx\n", pfx, proc->midr);
> +
> + len = proc->section_length - (sizeof(*proc) +
> + proc->err_info_num * (sizeof(*err_info)));
> + if (len < 0) {
> + printk("%ssection length is too small\n", pfx);
Now here we *can* dump it.
> + printk("%sfirmware-generated error record is incorrect\n", pfx);
> + printk("%sERR_INFO_NUM is %d\n", pfx, proc->err_info_num);
> + return;
> + }
> +
> + if (proc->validation_bits & CPER_ARM_VALID_MPIDR)
> + printk("%sMPIDR: 0x%016llx\n", pfx, proc->mpidr);
<---- newline here.
Also, what is MPIDR and can it be written in a more user-friendly manner
and not be an abbreviation?
> + if (proc->validation_bits & CPER_ARM_VALID_AFFINITY_LEVEL)
> + printk("%serror affinity level: %d\n", pfx,
> + proc->affinity_level);
> + if (proc->validation_bits & CPER_ARM_VALID_RUNNING_STATE) {
> + printk("%srunning state: 0x%x\n", pfx, proc->running_state);
> + printk("%sPSCI state: %d\n", pfx, proc->psci_state);
One more abbreviation. Please consider whether having the abbreviations
or actually writing them out is more user-friendly.
> + }
> +
> + snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
That INDENT_SP thing is just silly, someone should kill it.
> +
> + err_info = (struct cper_arm_err_info *)(proc + 1);
> + for (i = 0; i < proc->err_info_num; i++) {
> + printk("%sError info structure %d:\n", pfx, i);
> + printk("%sversion:%d\n", newpfx, err_info->version);
> + printk("%slength:%d\n", newpfx, err_info->length);
<---- newline here.
Why do we even dump version and info for *every* err_info structure?
> + if (err_info->validation_bits &
> + CPER_ARM_INFO_VALID_MULTI_ERR) {
> + if (err_info->multiple_error == 0)
> + printk("%ssingle error\n", newpfx);
> + else if (err_info->multiple_error == 1)
> + printk("%smultiple errors\n", newpfx);
> + else
> + printk("%smultiple errors count:%u\n",
> + newpfx, err_info->multiple_error);
So this can be simply: "num errors: %d", err_info->multiple_error+1...
Without checking CPER_ARM_INFO_VALID_MULTI_ERR.
> + }
<---- newline here.
> + if (err_info->validation_bits & CPER_ARM_INFO_VALID_FLAGS) {
> + if (err_info->flags & CPER_ARM_INFO_FLAGS_FIRST)
> + printk("%sfirst error captured\n", newpfx);
> + if (err_info->flags & CPER_ARM_INFO_FLAGS_LAST)
> + printk("%slast error captured\n", newpfx);
> + if (err_info->flags & CPER_ARM_INFO_FLAGS_PROPAGATED)
> + printk("%spropagated error captured\n",
> + newpfx);
> + if (err_info->flags & CPER_ARM_INFO_FLAGS_OVERFLOW)
> + printk("%soverflow occurred, error info is incomplete\n",
> + newpfx);
> + }
<---- newline here.
> + printk("%serror_type: %d, %s\n", newpfx, err_info->type,
> + err_info->type < ARRAY_SIZE(proc_error_type_strs) ?
> + proc_error_type_strs[err_info->type] : "unknown");
> + if (err_info->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO)
> + printk("%serror_info: 0x%016llx\n", newpfx,
> + err_info->error_info);
err_info->error_info ?
What is that supposed to mean? A u64 value of some sorts.
> + if (err_info->validation_bits & CPER_ARM_INFO_VALID_VIRT_ADDR)
> + printk("%svirtual fault address: 0x%016llx\n",
> + newpfx, err_info->virt_fault_addr);
> + if (err_info->validation_bits &
> + CPER_ARM_INFO_VALID_PHYSICAL_ADDR)
Just let that line stick out.
> + printk("%sphysical fault address: 0x%016llx\n",
> + newpfx, err_info->physical_fault_addr);
> + err_info += 1;
> + }
<---- newline here.
That function is kinda missing newlines.
> + ctx_info = (struct cper_arm_ctx_info *)err_info;
> + max_ctx_type = ARRAY_SIZE(arm_reg_ctx_strs) - 1;
> + for (i = 0; i < proc->context_info_num; i++) {
> + int size = sizeof(*ctx_info) + ctx_info->size;
> +
> + printk("%sContext info structure %d:\n", pfx, i);
> + if (len < size) {
> + printk("%ssection length is too small\n", newpfx);
> + printk("%sfirmware-generated error record is incorrect\n", pfx);
> + return;
> + }
> + if (ctx_info->type > max_ctx_type) {
> + printk("%sInvalid context type: %d\n", newpfx,
> + ctx_info->type);
> + printk("%sMax context type: %d\n", newpfx,
> + max_ctx_type);
> + return;
You can combine those into:
printk("%sInvalid context type: %d (max: %d)\n",
newpfx, ctx_info->type, max_ctx_type);
> + }
> + printk("%sregister context type %d: %s\n", newpfx,
> + ctx_info->type, arm_reg_ctx_strs[ctx_info->type]);
Why dump the type as %d and as a string too? String should be enough, no?
> + print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, 4,
> + (ctx_info + 1), ctx_info->size, 0);
> + len -= size;
> + ctx_info = (struct cper_arm_ctx_info *)((long)ctx_info + size);
> + }
> +
> + if (len > 0) {
> + printk("%sVendor specific error info has %u bytes:\n", pfx,
> + len);
> + print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, 4, ctx_info,
> + len, true);
That looks like it should be a debug printk...
> + }
> +}
> +#endif
> +
> static const char * const mem_err_type_strs[] = {
> "unknown",
> "no error",
> @@ -461,6 +586,16 @@ static void cper_estatus_timestamp(const char *pfx,
> cper_print_pcie(newpfx, pcie, gdata);
> else
> goto err_section_too_small;
> + } else if ((IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)) &&
> + !uuid_le_cmp(*sec_type, CPER_SEC_PROC_ARM)) {
> + struct cper_sec_proc_arm *arm_err;
> +
> + arm_err = acpi_hest_get_payload(gdata);
struct cper_sec_proc_arm *arm_err = acpi_hest_get_payload(gdata);
> + printk("%ssection_type: ARM processor error\n", newpfx);
> + if (gdata->error_data_length >= sizeof(*arm_err))
> + cper_print_proc_arm(newpfx, arm_err);
> + else
> + goto err_section_too_small;
You need to build-test your patches before submitting:
drivers/firmware/efi/cper.c: In function ?cper_estatus_print_section?:
drivers/firmware/efi/cper.c:596:4: error: implicit declaration of function ?cper_print_proc_arm? [-Werror=implicit-function-declaration]
cper_print_proc_arm(newpfx, arm_err);
^~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [drivers/firmware/efi/cper.o] Error 1
make[2]: *** [drivers/firmware/efi] Error 2
make[1]: *** [drivers/firmware] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2
make: *** Waiting for unfinished jobs....
this is a x86 build.
> } else
> printk("%s""section type: unknown, %pUl\n", newpfx, sec_type);
>
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index dcacb1a..85450f3 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -180,6 +180,10 @@ enum {
> #define CPER_SEC_PROC_IPF \
> UUID_LE(0xE429FAF1, 0x3CB7, 0x11D4, 0x0B, 0xCA, 0x07, 0x00, \
> 0x80, 0xC7, 0x3C, 0x88, 0x81)
> +/* Processor Specific: ARM */
> +#define CPER_SEC_PROC_ARM \
> + UUID_LE(0xE19E3D16, 0xBC11, 0x11E4, 0x9C, 0xAA, 0xC2, 0x05, \
> + 0x1D, 0x5D, 0x46, 0xB0)
> /* Platform Memory */
> #define CPER_SEC_PLATFORM_MEM \
> UUID_LE(0xA5BC1114, 0x6F64, 0x4EDE, 0xB8, 0x63, 0x3E, 0x83, \
> @@ -255,6 +259,22 @@ enum {
>
> #define CPER_PCIE_SLOT_SHIFT 3
>
> +#define CPER_ARM_VALID_MPIDR 0x00000001
> +#define CPER_ARM_VALID_AFFINITY_LEVEL 0x00000002
> +#define CPER_ARM_VALID_RUNNING_STATE 0x00000004
> +#define CPER_ARM_VALID_VENDOR_INFO 0x00000008
> +
> +#define CPER_ARM_INFO_VALID_MULTI_ERR 0x0001
> +#define CPER_ARM_INFO_VALID_FLAGS 0x0002
> +#define CPER_ARM_INFO_VALID_ERR_INFO 0x0004
> +#define CPER_ARM_INFO_VALID_VIRT_ADDR 0x0008
> +#define CPER_ARM_INFO_VALID_PHYSICAL_ADDR 0x0010
> +
> +#define CPER_ARM_INFO_FLAGS_FIRST 0x0001
> +#define CPER_ARM_INFO_FLAGS_LAST 0x0002
> +#define CPER_ARM_INFO_FLAGS_PROPAGATED 0x0004
> +#define CPER_ARM_INFO_FLAGS_OVERFLOW 0x0008
For all of the above use BIT().
> +
> /*
> * All tables and structs must be byte-packed to match CPER
> * specification, since the tables are provided by the system BIOS
> @@ -340,6 +360,40 @@ struct cper_ia_proc_ctx {
> __u64 mm_reg_addr;
> };
>
> +/* ARM Processor Error Section */
> +struct cper_sec_proc_arm {
> + __u32 validation_bits;
> + __u16 err_info_num; /* Number of Processor Error Info */
> + __u16 context_info_num; /* Number of Processor Context Info Records*/
> + __u32 section_length;
> + __u8 affinity_level;
> + __u8 reserved[3]; /* must be zero */
> + __u64 mpidr;
> + __u64 midr;
> + __u32 running_state; /* Bit 0 set - Processor running. PSCI = 0 */
> + __u32 psci_state;
Align comments vertically pls.
> +};
> +
> +/* ARM Processor Error Information Structure */
> +struct cper_arm_err_info {
> + __u8 version;
> + __u8 length;
> + __u16 validation_bits;
> + __u8 type;
> + __u16 multiple_error;
> + __u8 flags;
> + __u64 error_info;
> + __u64 virt_fault_addr;
> + __u64 physical_fault_addr;
> +};
> +
> +/* ARM Processor Context Information Structure */
> +struct cper_arm_ctx_info {
> + __u16 version;
> + __u16 type;
> + __u32 size;
> +};
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* [PATCH v2] arm64/dma-mapping: fix DMA_ATTR_FORCE_CONTIGUOUS mmaping code
From: Geert Uytterhoeven @ 2017-04-21 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421173233.GE5312@e104818-lin.cambridge.arm.com>
Hi Catalin,
On Fri, Apr 21, 2017 at 7:32 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Fri, Mar 31, 2017 at 01:02:51PM +0200, Andrzej Hajda wrote:
>> In case of DMA_ATTR_FORCE_CONTIGUOUS allocations vm_area->pages
>> is invalid. __iommu_mmap_attrs and __iommu_get_sgtable should not
>> use it and take advantage of contiguous nature of the allocation.
>
> As I replied to your original patch, I think
> dma_common_contiguous_remap() should be fixed not to leave a dangling
> area->pages pointer.
>
>> arch/arm64/mm/dma-mapping.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>> index f7b5401..41c7c36 100644
>> --- a/arch/arm64/mm/dma-mapping.c
>> +++ b/arch/arm64/mm/dma-mapping.c
>> @@ -703,6 +703,10 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
>> if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
>> return ret;
>>
>> + if (attrs & DMA_ATTR_FORCE_CONTIGUOUS)
>> + return vm_iomap_memory(vma,
>> + page_to_phys(vmalloc_to_page(cpu_addr)), size);
>
> I replied to Geert's patch on whether we actually need to call
> dma_common_contiguous_remap() in __iommu_alloc_attrs() if the device is
> coherent. We don't do this for the swiotlb allocation. If we are going
> to change this, cpu_addr may or may not be in the vmalloc range and
> vmalloc_to_page() would fail (I guess we could add an is_vmalloc_addr()
> check).
>
> Alternatively, just open-code dma_common_contiguous_remap() in
> __iommu_alloc_attrs() to keep a valid area->pages around (I think Geert
> already suggested this). AFAICT, arch/arm does this already in its own
> __iommu_alloc_buffer().
>
> Yet another option would be for iommu_dma_alloc() to understand
> DMA_ATTR_FORCE_CONTIGUOUS and remove the arm64-specific handling.
That was actually the approach I took in my v1.
V2 changed that to provide standalone iommu_dma_{alloc,free}_contiguous()
functions.
V3 changed that to call everything directly from the arm64 code.
...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support
From: Stefan Agner @ 2017-04-21 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57336d7e-7b48-8855-9e87-3eb370facd05@gmail.com>
On 2017-04-21 10:22, Marek Vasut wrote:
> On 04/21/2017 06:19 PM, Stefan Agner wrote:
>> On 2017-04-21 06:08, Marek Vasut wrote:
>>> On 04/21/2017 05:15 AM, Stefan Agner wrote:
>>>> On 2017-04-20 19:03, Marek Vasut wrote:
>>>>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>>>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>>>>> clock architecture requiring only two clocks to be referenced.
>>>>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>>>>> none of this differences are in use so there is no detection needed
>>>>>> and the driver can reuse IS_MX6SX.
>>>>>>
>>>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>>>> ---
>>>>>> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>>>> 1 file changed, 15 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>>>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>>>> .clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>>>> };
>>>>>>
>>>>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>>>>> + "gpmi_io", "gpmi_bch_apb",
>>>>>> +};
>>>>>> +
>>>>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>>>>> + .type = IS_MX6SX,
>>>>>
>>>>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>>>>
>>>>
>>>> Yeah I was thinking we can do it once we have an actual reason to
>>>> distinguish.
>>>
>>> So what are the differences anyway ?
>>>
>>
>> I did not check the details, but Han's patchset (link in cover letter)
>> mentions:
>> "add the HW bitflip detection and correction for i.MX6QP and i.MX7D."...
>
> Oh, interesting.
>
>>>> But then, adding the type would only require 2-3 lines of change if I
>>>> add it to the GPMI_IS_MX6 macro...
>>>
>>> Then at least add a comment because using type = IMX6SX right under
>>> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.
>>
>> FWIW, I mentioned it in the commit message.
>>
>> I think rather then adding a comment it is cleaner to just add IS_IMX7D
>> and add it to the GPMI_IS_MX6 macro. That does not need a comment since
>> it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a
>> rather small change. Does that sound acceptable?
>
> Sure, that's even better, thanks.
>
> btw isn't there some single-core mx7 (mx7s ?) , maybe we should just go
> with mx7 (without the d suffix). I dunno if it has GPMI NAND though, so
> maybe mx7d is the right thing to do here ...
>
There is a Solo version yes, and it has GPMI NAND too. However, almost
all i.MX 7 IPs have been named imx7d by NXP for some reason (including
compatible strings, see grep -r -e imx7 Documentation/), so I thought I
stay consistent here...
--
Stefan
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> @@ -132,7 +132,7 @@ static const char * const gpmi_clks_for_mx7d[] = {
>> };
>>
>> static const struct gpmi_devdata gpmi_devdata_imx7d = {
>> - .type = IS_MX6SX,
>> + .type = IS_MX7D,
>> .bch_max_ecc_strength = 62,
>> .max_chain_delay = 12,
>> .clks = gpmi_clks_for_mx7d,
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> index 2e584e18d980..f2cc13abc896 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> @@ -123,7 +123,8 @@ enum gpmi_type {
>> IS_MX23,
>> IS_MX28,
>> IS_MX6Q,
>> - IS_MX6SX
>> + IS_MX6SX,
>> + IS_MX7D,
>> };
>>
>> struct gpmi_devdata {
>> @@ -307,6 +308,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
>> #define GPMI_IS_MX28(x) ((x)->devdata->type == IS_MX28)
>> #define GPMI_IS_MX6Q(x) ((x)->devdata->type == IS_MX6Q)
>> #define GPMI_IS_MX6SX(x) ((x)->devdata->type == IS_MX6SX)
>> +#define GPMI_IS_MX7D(x) ((x)->devdata->type == IS_MX7D)
>>
>> -#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
>> +#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) ||
>> \
>> + GPMI_IS_MX7D(x))
>> #endif
>>
>> --
>> Stefan
>>
^ permalink raw reply
* [PATCH v2] arm64/dma-mapping: fix DMA_ATTR_FORCE_CONTIGUOUS mmaping code
From: Catalin Marinas @ 2017-04-21 17:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1490958171-2303-1-git-send-email-a.hajda@samsung.com>
On Fri, Mar 31, 2017 at 01:02:51PM +0200, Andrzej Hajda wrote:
> In case of DMA_ATTR_FORCE_CONTIGUOUS allocations vm_area->pages
> is invalid. __iommu_mmap_attrs and __iommu_get_sgtable should not
> use it and take advantage of contiguous nature of the allocation.
As I replied to your original patch, I think
dma_common_contiguous_remap() should be fixed not to leave a dangling
area->pages pointer.
> arch/arm64/mm/dma-mapping.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index f7b5401..41c7c36 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -703,6 +703,10 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
> if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
> return ret;
>
> + if (attrs & DMA_ATTR_FORCE_CONTIGUOUS)
> + return vm_iomap_memory(vma,
> + page_to_phys(vmalloc_to_page(cpu_addr)), size);
I replied to Geert's patch on whether we actually need to call
dma_common_contiguous_remap() in __iommu_alloc_attrs() if the device is
coherent. We don't do this for the swiotlb allocation. If we are going
to change this, cpu_addr may or may not be in the vmalloc range and
vmalloc_to_page() would fail (I guess we could add an is_vmalloc_addr()
check).
Alternatively, just open-code dma_common_contiguous_remap() in
__iommu_alloc_attrs() to keep a valid area->pages around (I think Geert
already suggested this). AFAICT, arch/arm does this already in its own
__iommu_alloc_buffer().
Yet another option would be for iommu_dma_alloc() to understand
DMA_ATTR_FORCE_CONTIGUOUS and remove the arm64-specific handling.
> @@ -715,8 +719,19 @@ static int __iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
> size_t size, unsigned long attrs)
> {
> unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> - struct vm_struct *area = find_vm_area(cpu_addr);
> + struct vm_struct *area;
> +
> + if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
> + int ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> +
> + if (!ret)
> + sg_set_page(sgt->sgl, vmalloc_to_page(cpu_addr),
> + PAGE_ALIGN(size), 0);
>
> + return ret;
> + }
> +
> + area = find_vm_area(cpu_addr);
As Russell already stated, this function needs a "fix" regardless of the
DMA_ATTR_FORCE_CONTIGUOUS as we may not have page structures
(*_from_coherent allocations). But I agree with you that
dma_get_sgtable() issues should be addressed separately (and I'm happy
to disable such API on arm64 until sorted).
--
Catalin
^ permalink raw reply
* [PATCH 1/1] PCI: Add MCFG quirk for 2nd node of Cavium ThunderX pass2.x host controller
From: Tomasz Nowicki @ 2017-04-21 17:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421165614.GA17916@bhelgaas-glaptop.roam.corp.google.com>
On 21.04.2017 18:56, Bjorn Helgaas wrote:
> On Wed, Mar 29, 2017 at 02:16:13PM +0200, Tomasz Nowicki wrote:
>> Currently SoCs pass2.x do not emulate EA headers for ACPI boot method at all.
>> However, for pass2.x some devices (like EDAC) advertise incorrect base addresses
>> in their BARs which results in driver probe failure during resource request.
>> Since all problematic blocks are on 2nd NUMA node under domain 10 add necessary
>> quirk entry to obtain BAR addresses correction using EA header emulation.
>>
>> Fixes: 44f22bd91e88 ("PCI: Add MCFG quirks for Cavium ThunderX pass2.x host controller")
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> CC: stable at vger.kernel.org # v4.10+
>
> Sorry Tomasz, I must have mistakenly marked this "accepted" in
> patchwork, so it fell off my to-do list. It's too late to get this in
> v4.11, but I can still apply it for v4.12.
>
> I put it on my pci/host-thunder branch with Robert's ack. I added a preceding
> patch to tidy up whitespace, which makes this patch look like just:
>
> /* SoC pass2.x */
> THUNDER_PEM_QUIRK(1, 0),
> THUNDER_PEM_QUIRK(1, 1),
> + THUNDER_ECAM_QUIRK(1, 10),
>
> Please double-check it to make sure this is what you need.
This is exactly what we need. Thank you Bjorn.
Tomasz
>
>> ---
>> drivers/acpi/pci_mcfg.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index a6a4cea..a0a391e 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -90,13 +90,14 @@ static struct mcfg_fixup mcfg_quirks[] = {
>> &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x894057000000UL, node) }, \
>> { "CAVIUM", "THUNDERX", rev, 9 + (10 * (node)), MCFG_BUS_ANY, \
>> &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x89808f000000UL, node) }
>> +#define THUNDER_ECAM_QUIRK(rev, seg) \
>> + { "CAVIUM", "THUNDERX", rev, seg, MCFG_BUS_ANY, \
>> + &pci_thunder_ecam_ops }
>> /* SoC pass2.x */
>> THUNDER_PEM_QUIRK(1, 0),
>> THUNDER_PEM_QUIRK(1, 1),
>> + THUNDER_ECAM_QUIRK(1, 10),
>>
>> -#define THUNDER_ECAM_QUIRK(rev, seg) \
>> - { "CAVIUM", "THUNDERX", rev, seg, MCFG_BUS_ANY, \
>> - &pci_thunder_ecam_ops }
>> /* SoC pass1.x */
>> THUNDER_PEM_QUIRK(2, 0), /* off-chip devices */
>> THUNDER_PEM_QUIRK(2, 1), /* off-chip devices */
>> --
>> 2.7.4
>>
^ permalink raw reply
* Applied "ASoC: mediatek: add I2C dependency for CS42XX8" to the asoc tree
From: Mark Brown @ 2017-04-21 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170420081541.658178-1-arnd@arndb.de>
The patch
ASoC: mediatek: add I2C dependency for CS42XX8
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 362c59436c143edbcdd4cc2ee0e9856eb6a93bc4 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 20 Apr 2017 10:15:20 +0200
Subject: [PATCH] ASoC: mediatek: add I2C dependency for CS42XX8
We should not select drivers that depend on I2C when that is disabled,
as it results in a build error:
warning: (SND_SOC_MT2701_WM8960) selects SND_SOC_WM8960 which has unmet direct dependencies (SOUND && !M68K && !UML && SND && SND_SOC && I2C)
sound/soc/codecs/wm8960.c:1469:1: error: data definition has no type or storage class [-Werror]
sound/soc/codecs/wm8960.c:1469:1: error: type defaults to 'int' in declaration of 'module_i2c_driver' [-Werror=implicit-int]
Fixes: 8625c1dbd876 ("ASoC: mediatek: Add mt2701-wm8960 machine driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/mediatek/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig
index c6f39040f71f..5c68797f36c4 100644
--- a/sound/soc/mediatek/Kconfig
+++ b/sound/soc/mediatek/Kconfig
@@ -24,7 +24,7 @@ config SND_SOC_MT2701_CS42448
config SND_SOC_MT2701_WM8960
tristate "ASoc Audio driver for MT2701 with WM8960 codec"
- depends on SND_SOC_MT2701
+ depends on SND_SOC_MT2701 && I2C
select SND_SOC_WM8960
help
This adds ASoC driver for Mediatek MT2701 boards
--
2.11.0
^ permalink raw reply related
* [PATCH V15 03/11] cper: add timestamp print to CPER status printing
From: Borislav Petkov @ 2017-04-21 17:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <35cc4ae0-e8fa-fd3a-5d7f-243ed2e82ea0@codeaurora.org>
On Fri, Apr 21, 2017 at 10:04:35AM -0600, Baicar, Tyler wrote:
> This is basically what I already had in v14...you asked to move it into a
> different if-statement? https://lkml.org/lkml/2017/4/12/397
Well, clearly I've been smoking some nasty potent sh*t. :-\
/me goes and looks at the spec:
"Bit 0 ? Timestamp is precise if this bit is set and correlates to the
time of the error event."
So why are we even printing the timestamp when !precise?
IOW, I think we should do:
if (!(timestamp[3] & 0x1))
printk("%stimestamp imprecise\n", pfx);
else {
sec = ..
min = ...
...
}
and print the actual values only when the timestamp is precise.
Otherwise it has *some* values which could just as well be completely
random. And it's not like we're reporting the error tomorrow - it is
mostly a couple of seconds from logging to the fw pushing it out...
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support
From: Marek Vasut @ 2017-04-21 17:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <494a7bc044457240a302d92b3a50b8e5@agner.ch>
On 04/21/2017 06:19 PM, Stefan Agner wrote:
> On 2017-04-21 06:08, Marek Vasut wrote:
>> On 04/21/2017 05:15 AM, Stefan Agner wrote:
>>> On 2017-04-20 19:03, Marek Vasut wrote:
>>>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>>>> clock architecture requiring only two clocks to be referenced.
>>>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>>>> none of this differences are in use so there is no detection needed
>>>>> and the driver can reuse IS_MX6SX.
>>>>>
>>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>>> ---
>>>>> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>>> 1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>>> .clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>>> };
>>>>>
>>>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>>>> + "gpmi_io", "gpmi_bch_apb",
>>>>> +};
>>>>> +
>>>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>>>> + .type = IS_MX6SX,
>>>>
>>>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>>>
>>>
>>> Yeah I was thinking we can do it once we have an actual reason to
>>> distinguish.
>>
>> So what are the differences anyway ?
>>
>
> I did not check the details, but Han's patchset (link in cover letter)
> mentions:
> "add the HW bitflip detection and correction for i.MX6QP and i.MX7D."...
Oh, interesting.
>>> But then, adding the type would only require 2-3 lines of change if I
>>> add it to the GPMI_IS_MX6 macro...
>>
>> Then at least add a comment because using type = IMX6SX right under
>> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.
>
> FWIW, I mentioned it in the commit message.
>
> I think rather then adding a comment it is cleaner to just add IS_IMX7D
> and add it to the GPMI_IS_MX6 macro. That does not need a comment since
> it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a
> rather small change. Does that sound acceptable?
Sure, that's even better, thanks.
btw isn't there some single-core mx7 (mx7s ?) , maybe we should just go
with mx7 (without the d suffix). I dunno if it has GPMI NAND though, so
maybe mx7d is the right thing to do here ...
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -132,7 +132,7 @@ static const char * const gpmi_clks_for_mx7d[] = {
> };
>
> static const struct gpmi_devdata gpmi_devdata_imx7d = {
> - .type = IS_MX6SX,
> + .type = IS_MX7D,
> .bch_max_ecc_strength = 62,
> .max_chain_delay = 12,
> .clks = gpmi_clks_for_mx7d,
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> index 2e584e18d980..f2cc13abc896 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> @@ -123,7 +123,8 @@ enum gpmi_type {
> IS_MX23,
> IS_MX28,
> IS_MX6Q,
> - IS_MX6SX
> + IS_MX6SX,
> + IS_MX7D,
> };
>
> struct gpmi_devdata {
> @@ -307,6 +308,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
> #define GPMI_IS_MX28(x) ((x)->devdata->type == IS_MX28)
> #define GPMI_IS_MX6Q(x) ((x)->devdata->type == IS_MX6Q)
> #define GPMI_IS_MX6SX(x) ((x)->devdata->type == IS_MX6SX)
> +#define GPMI_IS_MX7D(x) ((x)->devdata->type == IS_MX7D)
>
> -#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
> +#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) ||
> \
> + GPMI_IS_MX7D(x))
> #endif
>
> --
> Stefan
>
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode
From: Lorenzo Pieralisi @ 2017-04-21 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPEA6daR9+BmyW6G9XOsO18782a-tjJrJKyxcYNHeHonuXRggw@mail.gmail.com>
Hi Sergei,
On Fri, Apr 07, 2017 at 11:31:12AM -0700, Sergei Temerkhanov wrote:
> On Fri, Apr 7, 2017 at 10:18 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > Hi Sergey,
> >
> > On Thu, Apr 06, 2017 at 11:47:11AM +0000, Sergey Temerkhanov wrote:
> >> This commit modifies ARM64-specific ACPI PCI support functions to
> >> properly set the numa_node property on host bridges and thus on PCI
> >> devices. Such support requires _PXM objects to be set in DSDT/SSDT
> >> tables describing PCI root bridges
> >>
> >> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> >> ---
> >> arch/arm64/kernel/pci.c | 13 ++++++++++++-
> >> 1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> >> index 4f0e3ebfea4b..c45c7a26f984 100644
> >> --- a/arch/arm64/kernel/pci.c
> >> +++ b/arch/arm64/kernel/pci.c
> >> @@ -81,7 +81,16 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
> >>
> >> int pcibus_to_node(struct pci_bus *bus)
> >> {
> >> - return dev_to_node(&bus->dev);
> >> + struct pci_config_window *cfg = bus->sysdata;
> >
> > Well, this implicit cast is a guess (ie it might be a DT sysdata
> > and there you can't have a clue on what it really is).
>
> Yes, but it's only accessed when acpi_disabled != 0. I just tried to
> save few lines and brackets :-)
I have put together a patch, I will drop it on linux-pci next week
so that we can all have a look and test it.
Thanks !
Lorenzo
> Regards,
> Sergey
>
> >
> >> + struct acpi_device *adev = NULL;
> >> + struct device *dev;
> >> +
> >> + if (!acpi_disabled)
> >> + adev = to_acpi_device(cfg->parent);
> >> +
> >> + dev = adev ? &adev->dev : &bus->dev;
> >> +
> >> + return dev_to_node(dev);
> >> }
> >> EXPORT_SYMBOL(pcibus_to_node);
> >>
> >> @@ -186,6 +195,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> >> struct pci_bus *bus, *child;
> >> struct acpi_pci_root_ops *root_ops;
> >>
> >> + set_dev_node(&root->device->dev, node);
> >
> > Ok I suspect this is one of those catch-22 issues with host bridge
> > creation/scan code (like domain nr), let me have a look I will send
> > you a patch next week when I figure out the call ordering required
> > to make this work, I want to make sure we can make this work with
> > DT and ACPI alike.
> >
> > Thanks !
> > Lorenzo
> >
> >> +
> >> ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
> >> if (!ri)
> >> return NULL;
> >> --
> >> 2.12.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> >> the body of a message to majordomo at vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH V8 5/6] ACPI: Support the probing on the devices which apply indirect-IO
From: Lorenzo Pieralisi @ 2017-04-21 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a73c87d3-59fc-843b-a4e1-d9f5201bffd9@gmail.com>
On Fri, Apr 21, 2017 at 10:22:52AM +0800, zhichang.yuan wrote:
> Hi, Dann,
>
>
>
> On 04/21/2017 04:57 AM, dann frazier wrote:
> > On Thu, Mar 30, 2017 at 9:26 AM, zhichang.yuan
> > <yuanzhichang@hisilicon.com> wrote:
> >> On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access I/O
> >> with some special host-local I/O ports known on x86. To access the I/O
> >> peripherals, an indirect-IO mechanism is introduced to mapped the host-local
> >> I/O to system logical/fake PIO similar the PCI MMIO on architectures where no
> >> separate I/O space exists. Just as PCI MMIO, the host I/O range should be
> >> registered before probing the downstream devices and set up the I/O mapping.
> >> But current ACPI bus probing doesn't support these indirect-IO hosts/devices.
> >>
> >> This patch introdueces a new ACPI handler for this device category. Through the
> >> handler attach callback, the indirect-IO hosts I/O registration is done and
> >> all peripherals' I/O resources are translated into logic/fake PIO before
> >> starting the enumeration.
> >>
> >> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
> >> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> >> ---
> >> drivers/acpi/Makefile | 1 +
> >> drivers/acpi/acpi_indirectio.c | 344 +++++++++++++++++++++++++++++++++++++++++
> >> drivers/acpi/internal.h | 5 +
> >> drivers/acpi/scan.c | 1 +
> >> 4 files changed, 351 insertions(+)
> >> create mode 100644 drivers/acpi/acpi_indirectio.c
> >>
> >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> >> index a391bbc..10e5f2b 100644
> >> --- a/drivers/acpi/Makefile
> >> +++ b/drivers/acpi/Makefile
> >> @@ -57,6 +57,7 @@ acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
> >> acpi-y += acpi_lpat.o
> >> acpi-$(CONFIG_ACPI_GENERIC_GSI) += irq.o
> >> acpi-$(CONFIG_ACPI_WATCHDOG) += acpi_watchdog.o
> >> +acpi-$(CONFIG_INDIRECT_PIO) += acpi_indirectio.o
> >>
> >> # These are (potentially) separate modules
> >>
> >> diff --git a/drivers/acpi/acpi_indirectio.c b/drivers/acpi/acpi_indirectio.c
> >> new file mode 100644
> >> index 0000000..c8c80b5
> >> --- /dev/null
> >> +++ b/drivers/acpi/acpi_indirectio.c
> >> @@ -0,0 +1,344 @@
>
> [snip]
>
> >> +acpi_build_logiciores_template(struct acpi_device *adev,
> >> + struct acpi_buffer *buffer)
> >> +{
> >> + acpi_handle handle = adev->handle;
> >> + struct acpi_resource *resource;
> >> + acpi_status status;
> >> + int res_cnt = 0;
> >> +
> >> + status = acpi_walk_resources(handle, METHOD_NAME__CRS,
> >> + acpi_count_logiciores, &res_cnt);
> >> + if (ACPI_FAILURE(status) || !res_cnt) {
> >> + dev_err(&adev->dev, "can't evaluate _CRS: %d\n", status);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1;
> >> + buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);
> >
> > (Seth Forshee noticed this issue, just passing it on)
> >
> > Should this just allocate the full buffer->length? That would keep the
> > length attribute accurate (possibly avoiding an off-by-1 error later).
> > It's not clear what the trailing byte is needed for, but other drivers
> > allocate it as well (drivers/acpi/pci_link.c and
> > drivers/platform/x86/sony-laptop.c).
>
> Thanks for your suggestion!
>
> I also curious why this one appended byte is needed as it seems the later
> acpi_set_current_resources() doesn't use this byte.
> And I tested without setting the buffer->length as the length of resource list
> directly, it seems ok.
>
> But anyway, it looks more reasonable to allocate the memory with the
> buffer->length rather than buffer->length - 1;
>
> I was made the V9 patch-set, and I can add your suggestion there. But I also
> awaiting for ARM64 ACPI maintainer's comment about this patch before really
> sending V9. I wonder whether there is better way to make our indirect-IO devices
> can be assigned the logic PIO before the enumeration...
>
> Lorenzo, Hanjun, what do you think about this patch?
I will get to it shortly, sorry for the delay.
Thanks,
Lorenzo
^ permalink raw reply
* [PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling
From: Jayachandran C @ 2017-04-21 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAErSpo6+do8GXon3EzZAUpqc2TFsA0BDwWBULqZCd1ayCJhD6w@mail.gmail.com>
On Fri, Apr 21, 2017 at 10:48:15AM -0500, Bjorn Helgaas wrote:
> On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
> <jnair@caviumnetworks.com> wrote:
> > On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
> >> On Fri, Apr 14, 2017 at 4:06 PM, Jayachandran C
> >> <jnair@caviumnetworks.com> wrote:
> >> > On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
> >> >> I tentatively applied both patches to pci/host-thunder for v4.12.
> >> >>
> >> >> However, I am concerned about the topology here:
> >> >>
> >> >> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
> >> >> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
> >> >> > PCI topology is slightly unusual. For a multi-node system, it looks
> >> >> > like:
> >> >> >
> >> >> > 00:00.0 [PCI] bridge to [bus 01-1e]
> >> >> > 01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
> >> >> > 02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
> >> >> > 03:00.0 PCIe Endpoint
> >> >>
> >> >> A root port normally has a single PCIe link leading downstream.
> >> >> According to this, 02:00.0 is a root port that has the usual
> >> >> downstream link leading to 03:00.0, but it also has an upstream link
> >> >> to 01:0a.0.
> >> >
> >> > The PCI topology is a bit broken due to the way that the PCIe IP block
> >> > was integrated into SoC PCI bridges and devices. The current mechanism
> >> > of adding a PCI-PCIe bridge to glue these together is not ideal.
> >>
> >> Yeah, that's definitely broken.
> >>
> >> >> Maybe this example is omitting details that are not relevant to DMA
> >> >> aliases? The PCIe capability only contains one set of link-related
> >> >> registers, so I don't know how we could manage a single device that
> >> >> has two links.
> >> >
> >> > The root port is standard and has just one link to the EP (or whatever
> >> > is on the external PCIe slot). The fallout of the current hw design is
> >> > that the PCI-PCIe bridge has a link that does not follow standard and
> >> > does not have a counterpart (as you noted).
> >> >
> >> >> A device with two links would break things like ASPM. In
> >> >> set_pcie_port_type(), for example, we have this comment:
> >> >>
> >> >> * A Root Port or a PCI-to-PCIe bridge is always the upstream end
> >> >> * of a Link. No PCIe component has two Links. Two Links are
> >> >> * connected by a Switch that has a Port on each Link and internal
> >> >> * logic to connect the two Ports.
> >> >>
> >> >> The topology above breaks these assumptions, which will make
> >> >> pdev->has_secondary_link incorrect, which means ASPM won't work
> >> >> correctly.
> >> >
> >> > Given the current hardware, the pcieport driver seems to work reasonably
> >> > for the root port at 02:00.0, with AER support. I will take a look at the
> >> > ASPM part.
> >>
> >> I don't think pcieport itself cares much about links. ASPM does, but
> >> it looks like set_pcie_port_type() actually is smart enough to know
> >> that PCI-to-PCIe bridges and Root Ports always have links on their
> >> secondary sides. So has_secondary_link probably does get set
> >> correctly.
> >>
> >> But I think you'll hit the VIA "strange chipset" thing in
> >> pcie_aspm_init_link_state(), which will probably prevent us from doing
> >> ASPM on the link from 02:00.0 to 03:00.0.
> >>
> >> Could you collect "lspci -vv" output from this system? I'd like to
> >> archive that as background for this IOMMU issue and the ASPM tweaks I
> >> suspect we'll have to do. I *wish* we had more information about that
> >> VIA thing, because I suspect we could get rid of it if we had more
> >> details.
> >
> > The full logs are slightly large, so I have kept them at:
> > https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> > The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
> >
> > The output is from 2 socket system, the cards are not on the first slot
> > like the example above, so the bus and device numbers are different.
>
> Can somebody with this system collect the "lspci -xxxx" output as well?
>
> I'm making some lspci changes to handle the PCI-to-PCIe bridge
> correctly, and I can use the "lspci -xxxx" output to create an lspci
> test case.
[Sorry was AFK for a few days]
I have updated the above directory with the log. Also tested your next branch
and it works fine on ThunderX2.
JC.
^ permalink raw reply
* [PATCH 1/1] PCI: Add MCFG quirk for 2nd node of Cavium ThunderX pass2.x host controller
From: Bjorn Helgaas @ 2017-04-21 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1490789773-21506-1-git-send-email-tn@semihalf.com>
On Wed, Mar 29, 2017 at 02:16:13PM +0200, Tomasz Nowicki wrote:
> Currently SoCs pass2.x do not emulate EA headers for ACPI boot method at all.
> However, for pass2.x some devices (like EDAC) advertise incorrect base addresses
> in their BARs which results in driver probe failure during resource request.
> Since all problematic blocks are on 2nd NUMA node under domain 10 add necessary
> quirk entry to obtain BAR addresses correction using EA header emulation.
>
> Fixes: 44f22bd91e88 ("PCI: Add MCFG quirks for Cavium ThunderX pass2.x host controller")
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> CC: stable at vger.kernel.org # v4.10+
Sorry Tomasz, I must have mistakenly marked this "accepted" in
patchwork, so it fell off my to-do list. It's too late to get this in
v4.11, but I can still apply it for v4.12.
I put it on my pci/host-thunder branch with Robert's ack. I added a preceding
patch to tidy up whitespace, which makes this patch look like just:
/* SoC pass2.x */
THUNDER_PEM_QUIRK(1, 0),
THUNDER_PEM_QUIRK(1, 1),
+ THUNDER_ECAM_QUIRK(1, 10),
Please double-check it to make sure this is what you need.
> ---
> drivers/acpi/pci_mcfg.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index a6a4cea..a0a391e 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -90,13 +90,14 @@ static struct mcfg_fixup mcfg_quirks[] = {
> &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x894057000000UL, node) }, \
> { "CAVIUM", "THUNDERX", rev, 9 + (10 * (node)), MCFG_BUS_ANY, \
> &thunder_pem_ecam_ops, THUNDER_PEM_RES(0x89808f000000UL, node) }
> +#define THUNDER_ECAM_QUIRK(rev, seg) \
> + { "CAVIUM", "THUNDERX", rev, seg, MCFG_BUS_ANY, \
> + &pci_thunder_ecam_ops }
> /* SoC pass2.x */
> THUNDER_PEM_QUIRK(1, 0),
> THUNDER_PEM_QUIRK(1, 1),
> + THUNDER_ECAM_QUIRK(1, 10),
>
> -#define THUNDER_ECAM_QUIRK(rev, seg) \
> - { "CAVIUM", "THUNDERX", rev, seg, MCFG_BUS_ANY, \
> - &pci_thunder_ecam_ops }
> /* SoC pass1.x */
> THUNDER_PEM_QUIRK(2, 0), /* off-chip devices */
> THUNDER_PEM_QUIRK(2, 1), /* off-chip devices */
> --
> 2.7.4
>
^ permalink raw reply
* [GIT PULL 2/2] arm64 dts: exynos: Last round for v4.12
From: Krzysztof Kozlowski @ 2017-04-21 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421164050.7404-1-krzk@kernel.org>
Hi,
On top of previous pull request.
Best regards,
Krzysztof
The following changes since commit e3c07546747cdec07ff15c984bc6cebc9c9f788c:
arm64: dts: exynos: Add the burst and esc clock frequency properties to DSI node (2017-03-08 08:55:39 +0200)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt64-4.12-2
for you to fetch changes up to 5dccc9873c4af60f4478b3ef54267353f25cc301:
arm64: dts: exynos: Use - instead of @ for DT OPP entries (2017-04-20 18:10:05 +0200)
----------------------------------------------------------------
Second update of Samsung DeviceTree ARM64 for v4.12:
- Fix DTC warnings in Exynos ARM64 Device Tree sources.
- Add panel node to TM2E board.
----------------------------------------------------------------
Hoegeun Kwon (1):
arm64: dts: exynos: Add support for s6e3hf2 panel device on TM2e board
Viresh Kumar (1):
arm64: dts: exynos: Use - instead of @ for DT OPP entries
arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi | 48 ++++++++++++-------------
arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts | 11 ++++++
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 50 +++++++++++++-------------
3 files changed, 60 insertions(+), 49 deletions(-)
^ permalink raw reply
* [GIT PULL 1/2] ARM: dts: exynos: Last round for v4.12
From: Krzysztof Kozlowski @ 2017-04-21 16:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I know it is late so just one fix for v4.12.
On top of previous pull request.
Best regards,
Krzysztof
The following changes since commit a1146328ec47141007a6ec076e1cbfc80f31887f:
Merge tag 'samsung-dt-hdmi-cec-4.12' into next/dt (2017-03-31 00:27:18 +0300)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt-4.12-2
for you to fetch changes up to 6a611d149a9a1eff7c89e81bab018e4121cce17e:
ARM: dts: exynos: Use - instead of @ for DT OPP entries (2017-04-20 18:06:03 +0200)
----------------------------------------------------------------
Fix DTC warnings in Exynos ARMv7 Device Tree sources.
----------------------------------------------------------------
Viresh Kumar (1):
ARM: dts: exynos: Use - instead of @ for DT OPP entries
.../devicetree/bindings/devfreq/exynos-bus.txt | 46 +++++++--------
arch/arm/boot/dts/exynos3250.dtsi | 46 +++++++--------
arch/arm/boot/dts/exynos4210.dtsi | 32 +++++------
arch/arm/boot/dts/exynos4412-prime.dtsi | 4 +-
arch/arm/boot/dts/exynos4412.dtsi | 66 +++++++++++-----------
arch/arm/boot/dts/exynos5420.dtsi | 40 ++++++-------
arch/arm/boot/dts/exynos5800.dtsi | 56 +++++++++---------
.../boot/dts/exynos/exynos5433-tm2-common.dtsi | 43 +++++++++++++-
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 17 ++++++
arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts | 18 ++++++
10 files changed, 221 insertions(+), 147 deletions(-)
^ permalink raw reply
* [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support
From: Stefan Agner @ 2017-04-21 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8377dadc-043f-5932-cb13-3367db38a6dd@gmail.com>
On 2017-04-21 06:08, Marek Vasut wrote:
> On 04/21/2017 05:15 AM, Stefan Agner wrote:
>> On 2017-04-20 19:03, Marek Vasut wrote:
>>> On 04/21/2017 03:07 AM, Stefan Agner wrote:
>>>> Add support for i.MX 7 SoC. The i.MX 7 has a slightly different
>>>> clock architecture requiring only two clocks to be referenced.
>>>> The IP is slightly different compared to i.MX 6SoloX, but currently
>>>> none of this differences are in use so there is no detection needed
>>>> and the driver can reuse IS_MX6SX.
>>>>
>>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>>> ---
>>>> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++++++++++++
>>>> 1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>> index c8bbf5da2ab8..4a45d37ddc80 100644
>>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>>> @@ -127,6 +127,18 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>>>> .clks_count = ARRAY_SIZE(gpmi_clks_for_mx6),
>>>> };
>>>>
>>>> +static const char * const gpmi_clks_for_mx7d[] = {
>>>> + "gpmi_io", "gpmi_bch_apb",
>>>> +};
>>>> +
>>>> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
>>>> + .type = IS_MX6SX,
>>>
>>> Would it make sense to use IS_MX7 here already to prevent future surprises ?
>>>
>>
>> Yeah I was thinking we can do it once we have an actual reason to
>> distinguish.
>
> So what are the differences anyway ?
>
I did not check the details, but Han's patchset (link in cover letter)
mentions:
"add the HW bitflip detection and correction for i.MX6QP and i.MX7D."...
>> But then, adding the type would only require 2-3 lines of change if I
>> add it to the GPMI_IS_MX6 macro...
>
> Then at least add a comment because using type = IMX6SX right under
> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2.
FWIW, I mentioned it in the commit message.
I think rather then adding a comment it is cleaner to just add IS_IMX7D
and add it to the GPMI_IS_MX6 macro. That does not need a comment since
it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a
rather small change. Does that sound acceptable?
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -132,7 +132,7 @@ static const char * const gpmi_clks_for_mx7d[] = {
};
static const struct gpmi_devdata gpmi_devdata_imx7d = {
- .type = IS_MX6SX,
+ .type = IS_MX7D,
.bch_max_ecc_strength = 62,
.max_chain_delay = 12,
.clks = gpmi_clks_for_mx7d,
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 2e584e18d980..f2cc13abc896 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -123,7 +123,8 @@ enum gpmi_type {
IS_MX23,
IS_MX28,
IS_MX6Q,
- IS_MX6SX
+ IS_MX6SX,
+ IS_MX7D,
};
struct gpmi_devdata {
@@ -307,6 +308,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
#define GPMI_IS_MX28(x) ((x)->devdata->type == IS_MX28)
#define GPMI_IS_MX6Q(x) ((x)->devdata->type == IS_MX6Q)
#define GPMI_IS_MX6SX(x) ((x)->devdata->type == IS_MX6SX)
+#define GPMI_IS_MX7D(x) ((x)->devdata->type == IS_MX7D)
-#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
+#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) ||
\
+ GPMI_IS_MX7D(x))
#endif
--
Stefan
^ permalink raw reply related
* stable/linux-3.18.y build: 204 builds: 5 failed, 199 passed, 35 errors, 212 warnings (v3.18.49)
From: Arnd Bergmann @ 2017-04-21 16:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421145901.3wcft3ou7wwywuiu@sirena.org.uk>
On Fri, Apr 21, 2017 at 4:59 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Apr 21, 2017 at 04:27:14PM +0200, Arnd Bergmann wrote:
>> On Thu, Apr 20, 2017 at 7:04 PM, kernelci.org bot <bot@kernelci.org> wrote:
>> > stable/linux-3.18.y build: 204 builds: 5 failed, 199 passed, 35 errors, 212 warnings (v3.18.49)
>
>> > 2 arch/mips/mm/fault.c:321:1: error: the frame size of 1104 bytes is larger
>> > than 1024 bytes [-Werror=frame-larger-than=]
>
>> This is a result of a newer compiler version, combined with the -Werror
>> flag that is applied to arch/mips/, and two of the mips defconfigs overriding
>> CONFIG_FRAME_WARN to 1024 on a 64-bit architecture (probably by
>> accident).
>
> I'm wondering how good an idea it is to leave -Werror on in stable
> kernels given that they're very likely to get used with newer compilers
> at some point. But then I've never been a fan of -Werror in the first
> place.
I agree it's problematic as you never know what future compilers will warn
about. At the moment, alpha, mips, sh and sparc turn on -Werror
unconditionally for architecture specific code and there are only five
device drivers do this as well. The powerpc architecture and the i915
driver have a Kconfig option to control whether the warnings are enabled,
and they are turned off in allmodconfig.
I think this is just an artifact from the old days when we always had tons of
warnings in a given build, but we should try to replace it with something
better instead of just removing it (which would be easy enough to do).
I personally build with 'make CC="gcc -Werror" and fix all the warnings I
run into. I have some plans to improve it by hooking into the
scripts/Makefile.extrawarn infrastructure. Today, we can use "make W=1"
or "make W=12" to turn on extra levels of warnings, and I'd like to
add "make E=0" or "make E=012" as a way to turn on errors for
the default (0) or higher (1, 2, 3) levels, but this takes a little more
preparation.
Arnd
^ permalink raw reply
* [PATCH] arm64/dma-mapping: fix DMA_ATTR_FORCE_CONTIGUOUS mmaping code
From: Catalin Marinas @ 2017-04-21 16:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <15b1be13-625f-db74-d213-ad1df86f7eb5@arm.com>
On Wed, Mar 29, 2017 at 01:55:32PM +0100, Robin Murphy wrote:
> On 29/03/17 11:05, Andrzej Hajda wrote:
> > In case of DMA_ATTR_FORCE_CONTIGUOUS allocations vm_area->pages
> > is invalid. __iommu_mmap_attrs and __iommu_get_sgtable cannot use
> > it. In first case temporary pages array is passed to iommu_dma_mmap,
> > in 2nd case single entry sg table is created directly instead
> > of calling helper.
> >
> > Fixes: 44176bb ("arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU")
> > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> > ---
> > Hi,
> >
> > I am not familiar with this framework so please don't be too cruel ;)
> > Alternative solution I see is to always create vm_area->pages,
> > I do not know which approach is preferred.
> >
> > Regards
> > Andrzej
> > ---
> > arch/arm64/mm/dma-mapping.c | 40 ++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 38 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> > index f7b5401..bba2bc8 100644
> > --- a/arch/arm64/mm/dma-mapping.c
> > +++ b/arch/arm64/mm/dma-mapping.c
> > @@ -704,7 +704,30 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
> > return ret;
> >
> > area = find_vm_area(cpu_addr);
> > - if (WARN_ON(!area || !area->pages))
> > + if (WARN_ON(!area))
>
> From the look of things, it doesn't seem strictly necessary to change
> this, but whether that's a good thing is another matter. I'm not sure
> that dma_common_contiguous_remap() should really be leaving a dangling
> pointer in area->pages as it apparently does... :/
On this specific point, I don't think area->pages should be set either
(cc'ing Laura). As in the vmalloc vs vmap case, area->pages when pages
need to be freed (via vfree), which is not the case here. The
dma_common_pages_remap() would need to set area->pages when called
directly from the iommu DMA ops. Proposal below, not tested with the
iommu ops. I assume the patch would cause __iommu_mmap_attrs() to return
-ENXIO if DMA_ATTR_FORCE_CONTIGUOUS is set.
------------8<---------------------------------
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index efd71cf4fdea..ab7071041141 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -277,8 +277,8 @@ EXPORT_SYMBOL(dma_common_mmap);
* remaps an array of PAGE_SIZE pages into another vm_area
* Cannot be used in non-sleeping contexts
*/
-void *dma_common_pages_remap(struct page **pages, size_t size,
- unsigned long vm_flags, pgprot_t prot,
+static struct vm_struct *__dma_common_pages_remap(struct page **pages,
+ size_t size, unsigned long vm_flags, pgprot_t prot,
const void *caller)
{
struct vm_struct *area;
@@ -287,13 +287,26 @@ void *dma_common_pages_remap(struct page **pages, size_t size,
if (!area)
return NULL;
- area->pages = pages;
-
if (map_vm_area(area, prot, pages)) {
vunmap(area->addr);
return NULL;
}
+ return area;
+}
+
+void *dma_common_pages_remap(struct page **pages, size_t size,
+ unsigned long vm_flags, pgprot_t prot,
+ const void *caller)
+{
+ struct vm_struct *area;
+
+ area = __dma_common_pages_remap(pages, size, vm_flags, prot, caller);
+ if (!area)
+ return NULL;
+
+ area->pages = pages;
+
return area->addr;
}
@@ -308,7 +321,7 @@ void *dma_common_contiguous_remap(struct page *page, size_t size,
{
int i;
struct page **pages;
- void *ptr;
+ struct vm_struct *area;
unsigned long pfn;
pages = kmalloc(sizeof(struct page *) << get_order(size), GFP_KERNEL);
@@ -318,11 +331,13 @@ void *dma_common_contiguous_remap(struct page *page, size_t size,
for (i = 0, pfn = page_to_pfn(page); i < (size >> PAGE_SHIFT); i++)
pages[i] = pfn_to_page(pfn + i);
- ptr = dma_common_pages_remap(pages, size, vm_flags, prot, caller);
+ area = __dma_common_pages_remap(pages, size, vm_flags, prot, caller);
kfree(pages);
- return ptr;
+ if (!area)
+ return NULL;
+ return area->addr;
}
/*
--
Catalin
^ permalink raw reply related
* Touchscreen failure with CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
From: Fabio Estevam @ 2017-04-21 16:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Running 4.11-rc7 on a imx6q-sabresd board I notice that egalax
touchscreen stops generating evtest events after a random period of
time.
This problem can be avoided if I unselect CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND:
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -54,7 +54,6 @@ CONFIG_CMA=y
CONFIG_CMDLINE="noinitrd console=ttymxc0,115200"
CONFIG_KEXEC=y
CONFIG_CPU_FREQ=y
-CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
CONFIG_ARM_IMX6Q_CPUFREQ=y
CONFIG_CPU_IDLE=y
CONFIG_VFP=y
With this change evtest always capture all touchscreen events. No
single failure is seen.
I could see the same behavior with all mainline kernels I tested (4.9 and 4.10).
Any ideas as to how fix this bug when CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y?
Thanks
^ permalink raw reply
* [PATCH V15 03/11] cper: add timestamp print to CPER status printing
From: Baicar, Tyler @ 2017-04-21 16:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421122150.76cce2cfrt767glv@pd.tnic>
On 4/21/2017 6:21 AM, Borislav Petkov wrote:
> On Tue, Apr 18, 2017 at 05:05:15PM -0600, Tyler Baicar wrote:
>> The ACPI 6.1 spec added a timestamp to the HEST generic data
> HEST?
>
> I see the timestamp in
>
> Table 18-343 Generic Error Data Entry
>
> where those things are "One or more Generic Error Data Entry structures
> may be recorded in the Generic Error Data Entries field of the Generic
> Error Status Block structure."
>
> And those are part of the "18.3.2.7 Generic Hardware Error Source",
> i.e., GHES. So why do you say "HEST" above?
Ah yes, this should be GHES no HEST. There are too many acronyms
involved here :)
>
>> structure. Print the timestamp out when printing out the error
>> status information.
>>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>> CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>> Reviewed-by: James Morse <james.morse@arm.com>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Remove those Reviewed-by:s
>
>> ---
>> drivers/firmware/efi/cper.c | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
>> index 8328a6f..46585f9 100644
>> --- a/drivers/firmware/efi/cper.c
>> +++ b/drivers/firmware/efi/cper.c
>> @@ -32,6 +32,8 @@
>> #include <linux/acpi.h>
>> #include <linux/pci.h>
>> #include <linux/aer.h>
>> +#include <linux/printk.h>
>> +#include <linux/bcd.h>
>> #include <acpi/ghes.h>
>>
>> #define INDENT_SP " "
>> @@ -387,6 +389,29 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
>> pfx, pcie->bridge.secondary_status, pcie->bridge.control);
>> }
>>
>> +static void cper_estatus_timestamp(const char *pfx,
> cper_print_tstamp()
>
>> + struct acpi_hest_generic_data_v300 *gdata)
>> +{
>> + __u8 hour, min, sec, day, mon, year, century, *timestamp;
>> +
>> + if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
>> + timestamp = (__u8 *)&(gdata->time_stamp);
>> + sec = bcd2bin(timestamp[0]);
>> + min = bcd2bin(timestamp[1]);
>> + hour = bcd2bin(timestamp[2]);
>> + day = bcd2bin(timestamp[4]);
>> + mon = bcd2bin(timestamp[5]);
>> + year = bcd2bin(timestamp[6]);
>> + century = bcd2bin(timestamp[7]);
>> +
>> + if (*(timestamp + 3) & 0x1)
>> + printk("%stimestamp is precise\n", pfx);
>> +
>> + printk("%stime: %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
>> + century, year, mon, day, hour, min, sec);
> Yeah, about the precise tstamp, you can do something like this:
>
> ---
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 46585f92b741..a649884e2264 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -404,10 +404,8 @@ static void cper_estatus_timestamp(const char *pfx,
> year = bcd2bin(timestamp[6]);
> century = bcd2bin(timestamp[7]);
>
> - if (*(timestamp + 3) & 0x1)
> - printk("%stimestamp is precise\n", pfx);
> -
> - printk("%stime: %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
> + printk("%s%ststamp: %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
> + (timestamp[3] & 0x1 ? "precise " : ""),
> century, year, mon, day, hour, min, sec);
> }
> }
This is basically what I already had in v14...you asked to move it into
a different if-statement? https://lkml.org/lkml/2017/4/12/397
Thanks,
Tyler
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH V5 4/7] ARM: pxa: Use - instead of @ for DT OPP entries
From: Robert Jarzmik @ 2017-04-21 16:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421053041.GB26900@vireshk-i7>
Viresh Kumar <viresh.kumar@linaro.org> writes:
> On 20-04-17, 22:14, Robert Jarzmik wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>>
>> > Compiling the DT file with W=1, DTC warns like follows:
>> >
>> > Warning (unit_address_vs_reg): Node /opp_table0/opp at 1000000000 has a
>> > unit name, but no reg property
>> >
>> > Fix this by replacing '@' with '-' as the OPP nodes will never have a
>> > "reg" property.
>> >
>> > Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
>> > Reported-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> > Acked-by: Rob Herring <robh@kernel.org>
>> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
>
> Thanks. But I need you to pick it up for your pull request for arm-soc.
Sure, let me take it through my tree, no problem.
Cheers.
--
Robert
^ permalink raw reply
* [PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling
From: Bjorn Helgaas @ 2017-04-21 15:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170417174712.GA7575@localhost>
On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
<jnair@caviumnetworks.com> wrote:
> On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
>> On Fri, Apr 14, 2017 at 4:06 PM, Jayachandran C
>> <jnair@caviumnetworks.com> wrote:
>> > On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
>> >> I tentatively applied both patches to pci/host-thunder for v4.12.
>> >>
>> >> However, I am concerned about the topology here:
>> >>
>> >> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
>> >> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
>> >> > PCI topology is slightly unusual. For a multi-node system, it looks
>> >> > like:
>> >> >
>> >> > 00:00.0 [PCI] bridge to [bus 01-1e]
>> >> > 01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
>> >> > 02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
>> >> > 03:00.0 PCIe Endpoint
>> >>
>> >> A root port normally has a single PCIe link leading downstream.
>> >> According to this, 02:00.0 is a root port that has the usual
>> >> downstream link leading to 03:00.0, but it also has an upstream link
>> >> to 01:0a.0.
>> >
>> > The PCI topology is a bit broken due to the way that the PCIe IP block
>> > was integrated into SoC PCI bridges and devices. The current mechanism
>> > of adding a PCI-PCIe bridge to glue these together is not ideal.
>>
>> Yeah, that's definitely broken.
>>
>> >> Maybe this example is omitting details that are not relevant to DMA
>> >> aliases? The PCIe capability only contains one set of link-related
>> >> registers, so I don't know how we could manage a single device that
>> >> has two links.
>> >
>> > The root port is standard and has just one link to the EP (or whatever
>> > is on the external PCIe slot). The fallout of the current hw design is
>> > that the PCI-PCIe bridge has a link that does not follow standard and
>> > does not have a counterpart (as you noted).
>> >
>> >> A device with two links would break things like ASPM. In
>> >> set_pcie_port_type(), for example, we have this comment:
>> >>
>> >> * A Root Port or a PCI-to-PCIe bridge is always the upstream end
>> >> * of a Link. No PCIe component has two Links. Two Links are
>> >> * connected by a Switch that has a Port on each Link and internal
>> >> * logic to connect the two Ports.
>> >>
>> >> The topology above breaks these assumptions, which will make
>> >> pdev->has_secondary_link incorrect, which means ASPM won't work
>> >> correctly.
>> >
>> > Given the current hardware, the pcieport driver seems to work reasonably
>> > for the root port at 02:00.0, with AER support. I will take a look at the
>> > ASPM part.
>>
>> I don't think pcieport itself cares much about links. ASPM does, but
>> it looks like set_pcie_port_type() actually is smart enough to know
>> that PCI-to-PCIe bridges and Root Ports always have links on their
>> secondary sides. So has_secondary_link probably does get set
>> correctly.
>>
>> But I think you'll hit the VIA "strange chipset" thing in
>> pcie_aspm_init_link_state(), which will probably prevent us from doing
>> ASPM on the link from 02:00.0 to 03:00.0.
>>
>> Could you collect "lspci -vv" output from this system? I'd like to
>> archive that as background for this IOMMU issue and the ASPM tweaks I
>> suspect we'll have to do. I *wish* we had more information about that
>> VIA thing, because I suspect we could get rid of it if we had more
>> details.
>
> The full logs are slightly large, so I have kept them at:
> https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
>
> The output is from 2 socket system, the cards are not on the first slot
> like the example above, so the bus and device numbers are different.
Can somebody with this system collect the "lspci -xxxx" output as well?
I'm making some lspci changes to handle the PCI-to-PCIe bridge
correctly, and I can use the "lspci -xxxx" output to create an lspci
test case.
^ permalink raw reply
* [PATCHv4 00/12] Ion cleanup in preparation for moving out of staging
From: Sumit Semwal @ 2017-04-21 15:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492540034-5466-1-git-send-email-labbott@redhat.com>
Hi Laura,
Thanks much for this series!
On 18 April 2017 at 23:57, Laura Abbott <labbott@redhat.com> wrote:
> Hi,
>
> This is v4 of the series to cleanup to Ion. Greg took some of the patches
> that weren't CMA related already. There was a minor bisectability problem
> with the CMA APIs so this is a new version to address that. I also
> addressed some minor comments on the patch to collapse header files.
For the series, please feel free to apply my
Acked-by: Sumit Semwal <sumit.semwal@linaro.org>
>
> Thanks,
> Laura
>
> Laura Abbott (12):
> cma: Store a name in the cma structure
> cma: Introduce cma_for_each_area
> staging: android: ion: Use CMA APIs directly
> staging: android: ion: Stop butchering the DMA address
> staging: android: ion: Break the ABI in the name of forward progress
> staging: android: ion: Get rid of ion_phys_addr_t
> staging: android: ion: Collapse internal header files
> staging: android: ion: Rework heap registration/enumeration
> staging: android: ion: Drop ion_map_kernel interface
> staging: android: ion: Remove ion_handle and ion_client
> staging: android: ion: Set query return value
> staging/android: Update Ion TODO list
>
> arch/powerpc/kvm/book3s_hv_builtin.c | 3 +-
> drivers/base/dma-contiguous.c | 5 +-
> drivers/staging/android/TODO | 21 +-
> drivers/staging/android/ion/Kconfig | 32 +
> drivers/staging/android/ion/Makefile | 11 +-
> drivers/staging/android/ion/compat_ion.c | 152 -----
> drivers/staging/android/ion/compat_ion.h | 29 -
> drivers/staging/android/ion/ion-ioctl.c | 55 +-
> drivers/staging/android/ion/ion.c | 812 ++----------------------
> drivers/staging/android/ion/ion.h | 386 ++++++++---
> drivers/staging/android/ion/ion_carveout_heap.c | 21 +-
> drivers/staging/android/ion/ion_chunk_heap.c | 16 +-
> drivers/staging/android/ion/ion_cma_heap.c | 120 ++--
> drivers/staging/android/ion/ion_heap.c | 68 --
> drivers/staging/android/ion/ion_page_pool.c | 3 +-
> drivers/staging/android/ion/ion_priv.h | 453 -------------
> drivers/staging/android/ion/ion_system_heap.c | 39 +-
> drivers/staging/android/uapi/ion.h | 36 +-
> include/linux/cma.h | 6 +-
> mm/cma.c | 31 +-
> mm/cma.h | 1 +
> mm/cma_debug.c | 2 +-
> 22 files changed, 524 insertions(+), 1778 deletions(-)
> delete mode 100644 drivers/staging/android/ion/compat_ion.c
> delete mode 100644 drivers/staging/android/ion/compat_ion.h
> delete mode 100644 drivers/staging/android/ion/ion_priv.h
>
> --
> 2.7.4
>
--
Thanks and regards,
Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org ? Open source software for ARM SoCs
^ permalink raw reply
* [linux-sunxi] [PATCH 13/15] drm/sun4i: Add HDMI support
From: Chen-Yu Tsai @ 2017-04-21 15:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c63d01aedeccc58bf8d6f5bfd66d8595156e9491.1488876832.git-series.maxime.ripard@free-electrons.com>
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI
> controller.
>
> That HDMI controller is able to do audio and CEC, but those have been left
> out for now.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/Makefile | 5 +-
> drivers/gpu/drm/sun4i/sun4i_hdmi.h | 124 ++++++-
> drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 128 ++++++-
> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 449 +++++++++++++++++++++-
> drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++++++++++-
> 5 files changed, 942 insertions(+), 0 deletions(-)
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
Applying patch #9608371 using 'git am'
Description: [13/15] drm/sun4i: Add HDMI support
Applying: drm/sun4i: Add HDMI support
.git/rebase-apply/patch:116: trailing whitespace.
.git/rebase-apply/patch:531: trailing whitespace.
.git/rebase-apply/patch:701: trailing whitespace.
warning: 3 lines add whitespace errors.
> diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
> index 59b757350a1f..68a0f6244a59 100644
> --- a/drivers/gpu/drm/sun4i/Makefile
> +++ b/drivers/gpu/drm/sun4i/Makefile
> @@ -7,7 +7,12 @@ sun4i-tcon-y += sun4i_dotclock.o
> sun4i-tcon-y += sun4i_crtc.o
> sun4i-tcon-y += sun4i_layer.o
>
> +sun4i-drm-hdmi-y += sun4i_hdmi_enc.o
> +sun4i-drm-hdmi-y += sun4i_hdmi_ddc_clk.o
> +sun4i-drm-hdmi-y += sun4i_hdmi_tmds_clk.o
> +
> obj-$(CONFIG_DRM_SUN4I) += sun4i-drm.o sun4i-tcon.o
> obj-$(CONFIG_DRM_SUN4I) += sun4i_backend.o
> obj-$(CONFIG_DRM_SUN4I) += sun6i_drc.o
> +obj-$(CONFIG_DRM_SUN4I) += sun4i-drm-hdmi.o
> obj-$(CONFIG_DRM_SUN4I) += sun4i_tv.o
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
> new file mode 100644
> index 000000000000..2ad25b8fd3cd
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
> @@ -0,0 +1,124 @@
> +/*
> + * Copyright (C) 2016 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#ifndef _SUN4I_HDMI_H_
> +#define _SUN4I_HDMI_H_
> +
> +#include <drm/drm_connector.h>
> +#include <drm/drm_encoder.h>
> +
> +#define SUN4I_HDMI_CTRL_REG 0x004
> +#define SUN4I_HDMI_CTRL_ENABLE BIT(31)
> +
> +#define SUN4I_HDMI_IRQ_REG 0x008
> +#define SUN4I_HDMI_IRQ_STA_MASK 0x73
> +#define SUN4I_HDMI_IRQ_STA_FIFO_OF BIT(1)
> +#define SUN4I_HDMI_IRQ_STA_FIFO_UF BIT(0)
> +
> +#define SUN4I_HDMI_HPD_REG 0x00c
> +#define SUN4I_HDMI_HPD_HIGH BIT(0)
> +
> +#define SUN4I_HDMI_VID_CTRL_REG 0x010
> +#define SUN4I_HDMI_VID_CTRL_ENABLE BIT(31)
> +#define SUN4I_HDMI_VID_CTRL_HDMI_MODE BIT(30)
> +
> +#define SUN4I_HDMI_VID_TIMING_ACT_REG 0x014
> +#define SUN4I_HDMI_VID_TIMING_BP_REG 0x018
> +#define SUN4I_HDMI_VID_TIMING_FP_REG 0x01c
> +#define SUN4I_HDMI_VID_TIMING_SPW_REG 0x020
> +
> +#define SUN4I_HDMI_VID_TIMING_X(x) ((((x) - 1) & GENMASK(11, 0)))
> +#define SUN4I_HDMI_VID_TIMING_Y(y) ((((y) - 1) & GENMASK(11, 0)) << 16)
> +
> +#define SUN4I_HDMI_VID_TIMING_POL_REG 0x024
> +#define SUN4I_HDMI_VID_TIMING_POL_TX_CLK (0x3e0 << 16)
> +#define SUN4I_HDMI_VID_TIMING_POL_VSYNC BIT(1)
> +#define SUN4I_HDMI_VID_TIMING_POL_HSYNC BIT(0)
> +
> +#define SUN4I_HDMI_AVI_INFOFRAME_REG(n) (0x080 + (n))
> +
> +#define SUN4I_HDMI_PAD_CTRL0_REG 0x200
> +
> +#define SUN4I_HDMI_PAD_CTRL1_REG 0x204
> +#define SUN4I_HDMI_PAD_CTRL1_HALVE_CLK BIT(6)
> +
> +#define SUN4I_HDMI_PLL_CTRL_REG 0x208
> +#define SUN4I_HDMI_PLL_CTRL_DIV(n) ((n) << 4)
> +#define SUN4I_HDMI_PLL_CTRL_DIV_MASK GENMASK(7, 4)
> +
> +#define SUN4I_HDMI_PLL_DBG0_REG 0x20c
> +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(n) (((n) & 1) << 21)
> +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK BIT(21)
> +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT 21
> +
> +#define SUN4I_HDMI_PKT_CTRL_REG(n) (0x2f0 + (4 * (n)))
> +#define SUN4I_HDMI_PKT_CTRL_TYPE(n, t) ((t) << (((n) % 4) * 4))
> +
> +#define SUN4I_HDMI_UNKNOWN_REG 0x300
> +#define SUN4I_HDMI_UNKNOWN_INPUT_SYNC BIT(27)
> +
> +#define SUN4I_HDMI_DDC_CTRL_REG 0x500
> +#define SUN4I_HDMI_DDC_CTRL_ENABLE BIT(31)
> +#define SUN4I_HDMI_DDC_CTRL_START_CMD BIT(30)
> +#define SUN4I_HDMI_DDC_CTRL_RESET BIT(0)
> +
> +#define SUN4I_HDMI_DDC_ADDR_REG 0x504
> +#define SUN4I_HDMI_DDC_ADDR_SEGMENT(seg) (((seg) & 0xff) << 24)
> +#define SUN4I_HDMI_DDC_ADDR_EDDC(addr) (((addr) & 0xff) << 16)
> +#define SUN4I_HDMI_DDC_ADDR_OFFSET(off) (((off) & 0xff) << 8)
> +#define SUN4I_HDMI_DDC_ADDR_SLAVE(addr) ((addr) & 0xff)
> +
> +#define SUN4I_HDMI_DDC_FIFO_CTRL_REG 0x510
> +#define SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR BIT(31)
> +
> +#define SUN4I_HDMI_DDC_FIFO_DATA_REG 0x518
> +#define SUN4I_HDMI_DDC_BYTE_COUNT_REG 0x51c
> +
> +#define SUN4I_HDMI_DDC_CMD_REG 0x520
> +#define SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ 6
> +
> +#define SUN4I_HDMI_DDC_CLK_REG 0x528
> +#define SUN4I_HDMI_DDC_CLK_M(m) (((m) & 0x7) << 3)
> +#define SUN4I_HDMI_DDC_CLK_N(n) ((n) & 0x7)
> +
> +#define SUN4I_HDMI_DDC_LINE_CTRL_REG 0x540
> +#define SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE BIT(9)
> +#define SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE BIT(8)
> +
> +#define SUN4I_HDMI_DDC_FIFO_SIZE 16
> +
> +enum sun4i_hdmi_pkt_type {
> + SUN4I_HDMI_PKT_AVI = 2,
> + SUN4I_HDMI_PKT_END = 15,
> +};
> +
> +struct sun4i_hdmi {
> + struct drm_connector connector;
> + struct drm_encoder encoder;
> + struct device *dev;
> +
> + void __iomem *base;
> + struct clk *bus_clk;
> + struct clk *ddc_clk;
> + struct clk *mod_clk;
> + struct clk *pll0_clk;
> + struct clk *pll1_clk;
> + struct clk *tmds_clk;
> +
> + struct sun4i_drv *drv;
> +
> + bool hdmi_monitor;
> +};
> +
> +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *clk);
> +int sun4i_tmds_create(struct sun4i_hdmi *hdmi);
> +
> +#endif /* _SUN4I_HDMI_H_ */
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
> new file mode 100644
> index 000000000000..5125b14ea7a5
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
> @@ -0,0 +1,128 @@
> +/*
> + * Copyright (C) 2016 Free Electrons
> + * Copyright (C) 2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "sun4i_tcon.h"
> +#include "sun4i_hdmi.h"
> +
> +struct sun4i_ddc {
> + struct clk_hw hw;
> + struct sun4i_hdmi *hdmi;
> +};
> +
> +static inline struct sun4i_ddc *hw_to_ddc(struct clk_hw *hw)
> +{
> + return container_of(hw, struct sun4i_ddc, hw);
> +}
> +
> +static unsigned long sun4i_ddc_calc_divider(unsigned long rate,
> + unsigned long parent_rate,
> + u8 *m, u8 *n)
> +{
> + unsigned long best_rate = 0;
> + u8 best_m = 0, best_n = 0, _m, _n;
> +
> + for (_m = 0; _m < 8; _m++) {
> + for (_n = 0; _n < 8; _n++) {
> + unsigned long tmp_rate;
> +
> + tmp_rate = (((parent_rate / 2) / 10) >> _n) / (_m + 1);
> +
> + if (tmp_rate > rate)
> + continue;
> +
> + if (abs(rate - tmp_rate) < abs(rate - best_rate)) {
> + best_rate = tmp_rate;
> + best_m = _m;
> + best_n = _n;
> + }
> + }
> + }
> +
> + if (m && n) {
> + *m = best_m;
> + *n = best_n;
> + }
> +
> + return best_rate;
> +}
> +
> +static long sun4i_ddc_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + return sun4i_ddc_calc_divider(rate, *prate, NULL, NULL);
> +}
> +
> +static unsigned long sun4i_ddc_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct sun4i_ddc *ddc = hw_to_ddc(hw);
> + u32 reg;
> + u8 m, n;
> +
> + reg = readl(ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG);
> + m = (reg >> 3) & 0x7;
> + n = reg & 0x7;
> +
> + return (((parent_rate / 2) / 10) >> n) / (m + 1);
> +}
> +
> +static int sun4i_ddc_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct sun4i_ddc *ddc = hw_to_ddc(hw);
> + u8 div_m, div_n;
> +
> + sun4i_ddc_calc_divider(rate, parent_rate, &div_m, &div_n);
> +
> + writel(SUN4I_HDMI_DDC_CLK_M(div_m) | SUN4I_HDMI_DDC_CLK_N(div_n),
> + ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops sun4i_ddc_ops = {
> + .recalc_rate = sun4i_ddc_recalc_rate,
> + .round_rate = sun4i_ddc_round_rate,
> + .set_rate = sun4i_ddc_set_rate,
> +};
> +
> +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent)
> +{
> + struct clk_init_data init;
> + struct sun4i_ddc *ddc;
> + const char *parent_name;
> +
> + parent_name = __clk_get_name(parent);
> + if (!parent_name)
> + return -ENODEV;
> +
> + ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);
> + if (!ddc)
> + return -ENOMEM;
> +
> + init.name = "hdmi-ddc";
> + init.ops = &sun4i_ddc_ops;
> + init.parent_names = &parent_name;
> + init.num_parents = 1;
> + init.flags = CLK_SET_RATE_PARENT;
I don't think this is really needed. It probably doesn't hurt though,
since DDC is used when HDMI is not used for displaying, but it might
affect any upstream PLLs, which theoretically may affect other users
of said PLLs. The DDC clock is slow enough that we should be able to
generate a usable clock rate anyway.
> +
> + ddc->hdmi = hdmi;
> + ddc->hw.init = &init;
> +
> + hdmi->ddc_clk = devm_clk_register(hdmi->dev, &ddc->hw);
> + if (IS_ERR(hdmi->ddc_clk))
> + return PTR_ERR(hdmi->ddc_clk);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> new file mode 100644
> index 000000000000..33175308c2ed
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> @@ -0,0 +1,449 @@
> +/*
> + * Copyright (C) 2016 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_panel.h>
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "sun4i_backend.h"
> +#include "sun4i_drv.h"
> +#include "sun4i_hdmi.h"
> +#include "sun4i_tcon.h"
> +
> +static inline struct sun4i_hdmi *
> +drm_encoder_to_sun4i_hdmi(struct drm_encoder *encoder)
> +{
> + return container_of(encoder, struct sun4i_hdmi,
> + encoder);
> +}
> +
> +static inline struct sun4i_hdmi *
> +drm_connector_to_sun4i_hdmi(struct drm_connector *connector)
> +{
> + return container_of(connector, struct sun4i_hdmi,
> + connector);
> +}
> +
> +static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
> + struct drm_display_mode *mode)
> +{
> + struct hdmi_avi_infoframe frame;
> + u8 buffer[17];
> + int i, ret;
> +
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + if (ret < 0) {
> + DRM_ERROR("Failed to get infoframes from mode\n");
> + return ret;
> + }
> +
> + ret = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));
> + if (ret < 0) {
> + DRM_ERROR("Failed to pack infoframes\n");
> + return ret;
> + }
> +
> + for (i = 0; i < sizeof(buffer); i++)
> + writeb(buffer[i], hdmi->base + SUN4I_HDMI_AVI_INFOFRAME_REG(i));
> +
> + return 0;
> +}
> +
> +static void sun4i_hdmi_disable(struct drm_encoder *encoder)
> +{
> + struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> + struct sun4i_drv *drv = hdmi->drv;
> + struct sun4i_tcon *tcon = drv->tcon;
> + u32 val;
> +
> + DRM_DEBUG_DRIVER("Disabling the HDMI Output\n");
> +
> + val = readl(hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
> + val &= ~SUN4I_HDMI_VID_CTRL_ENABLE;
> + writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
> +
> + sun4i_tcon_channel_disable(tcon, 1);
> +}
> +
> +static void sun4i_hdmi_enable(struct drm_encoder *encoder)
> +{
> + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> + struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> + struct sun4i_drv *drv = hdmi->drv;
> + struct sun4i_tcon *tcon = drv->tcon;
> + u32 val = 0;
> +
> + DRM_DEBUG_DRIVER("Enabling the HDMI Output\n");
> +
> + sun4i_tcon_channel_enable(tcon, 1);
> +
> + sun4i_hdmi_setup_avi_infoframes(hdmi, mode);
> + val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);
> + val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END);
> + writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0));
> +
> + val = SUN4I_HDMI_VID_CTRL_ENABLE;
> + if (hdmi->hdmi_monitor)
> + val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;
> +
> + writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
> +}
> +
> +static void sun4i_hdmi_mode_set(struct drm_encoder *encoder,
> + struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode)
> +{
> + struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> + struct sun4i_drv *drv = hdmi->drv;
> + struct sun4i_tcon *tcon = drv->tcon;
> + unsigned int x, y;
> + u32 val;
> +
> + sun4i_tcon1_mode_set(tcon, encoder, mode);
> + clk_set_rate(tcon->sclk1, mode->crtc_clock * 1000);
> + clk_set_rate(hdmi->tmds_clk, mode->crtc_clock * 1000);
> +
> + /* Set input sync enable */
> + writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC,
> + hdmi->base + SUN4I_HDMI_UNKNOWN_REG);
> +
> + /* Setup timing registers */
> + writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |
> + SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),
> + hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);
> +
> + x = mode->htotal - mode->hsync_start;
> + y = mode->vtotal - mode->vsync_start;
I'm a bit skeptical about this one. All the other parameters are not
inclusive of other, why would this one be different? Shouldn't it
be "Xtotal - Xsync_end" instead?
> + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> + hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);
> +
> + x = mode->hsync_start - mode->hdisplay;
> + y = mode->vsync_start - mode->vdisplay;
> + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> + hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);
> +
> + x = mode->hsync_end - mode->hsync_start;
> + y = mode->vsync_end - mode->vsync_start;
> + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> + hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);
> +
> + val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;
> + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> + val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;
> +
> + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> + val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;
> +
> + writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);
You don't handle the interlaced video here, even though you set
hdmi->connector.interlace_allowed = true
later.
The double clock and double scan flags aren't handled either, though
I don't understand which one is supposed to represent the need for the
HDMI pixel repeater. AFAIK this is required for resolutions with pixel
clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.
> +}
> +
> +static struct drm_encoder_helper_funcs sun4i_hdmi_helper_funcs = {
> + .disable = sun4i_hdmi_disable,
> + .enable = sun4i_hdmi_enable,
> + .mode_set = sun4i_hdmi_mode_set,
> +};
> +
> +static struct drm_encoder_funcs sun4i_hdmi_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> +static int sun4i_hdmi_read_sub_block(struct sun4i_hdmi *hdmi,
> + unsigned int blk, unsigned int offset,
> + u8 *buf, unsigned int count)
> +{
> + unsigned long reg;
> + int i;
> +
> + reg = readl(hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);
> + writel(reg | SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR,
> + hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);
> + writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) |
> + SUN4I_HDMI_DDC_ADDR_EDDC(0x60) |
> + SUN4I_HDMI_DDC_ADDR_OFFSET(offset) |
> + SUN4I_HDMI_DDC_ADDR_SLAVE(0x50),
You can use DDC_ADDR from drm_edid.h.
> + hdmi->base + SUN4I_HDMI_DDC_ADDR_REG);
> +
> + writel(count, hdmi->base + SUN4I_HDMI_DDC_BYTE_COUNT_REG);
> + writel(SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ,
> + hdmi->base + SUN4I_HDMI_DDC_CMD_REG);
> +
> + reg = readl(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);
> + writel(reg | SUN4I_HDMI_DDC_CTRL_START_CMD,
> + hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);
> +
> + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg,
> + !(reg & SUN4I_HDMI_DDC_CTRL_START_CMD),
> + 100, 2000))
> + return -EIO;
> +
> + for (i = 0; i < count; i++)
> + buf[i] = readb(hdmi->base + SUN4I_HDMI_DDC_FIFO_DATA_REG);
> +
> + return 0;
> +}
> +
> +static int sun4i_hdmi_read_edid_block(void *data, u8 *buf, unsigned int blk,
> + size_t length)
> +{
> + struct sun4i_hdmi *hdmi = data;
> + int retry = 2, i;
> +
> + do {
> + for (i = 0; i < length; i += SUN4I_HDMI_DDC_FIFO_SIZE) {
> + unsigned char offset = blk * EDID_LENGTH + i;
> + unsigned int count = min((unsigned int)SUN4I_HDMI_DDC_FIFO_SIZE,
> + length - i);
> + int ret;
> +
> + ret = sun4i_hdmi_read_sub_block(hdmi, blk, offset,
> + buf + i, count);
> + if (ret)
> + return ret;
> + }
> + } while (!drm_edid_block_valid(buf, blk, true, NULL) && (retry--));
> +
> + return 0;
> +}
> +
> +static int sun4i_hdmi_get_modes(struct drm_connector *connector)
> +{
> + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
> + unsigned long reg;
> + struct edid *edid;
> + int ret;
> +
> + /* Reset i2c controller */
> + writel(SUN4I_HDMI_DDC_CTRL_ENABLE | SUN4I_HDMI_DDC_CTRL_RESET,
> + hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);
> + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg,
> + !(reg & SUN4I_HDMI_DDC_CTRL_RESET),
> + 100, 2000))
> + return -EIO;
> +
> + writel(SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE |
> + SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE,
> + hdmi->base + SUN4I_HDMI_DDC_LINE_CTRL_REG);
> +
> + clk_set_rate(hdmi->ddc_clk, 100000);
> +
> + edid = drm_do_get_edid(connector, sun4i_hdmi_read_edid_block, hdmi);
> + if (!edid)
> + return 0;
> +
> + hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
> + DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
> + hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
> +
> + drm_mode_connector_update_edid_property(connector, edid);
> + ret = drm_add_edid_modes(connector, edid);
> + kfree(edid);
> +
> + return ret;
> +}
> +
> +static struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = {
> + .get_modes = sun4i_hdmi_get_modes,
> +};
> +
> +static enum drm_connector_status
> +sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force)
> +{
> + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
> + unsigned long reg;
> +
> + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
> + reg & SUN4I_HDMI_HPD_HIGH,
> + 0, 500000))
We shouldn't need to do polling here. It should just return the status
at the instance it's called. Instead we should have a worker that does
polling to check if something is plugged or unplugged. I don't see any
interrupt bits for this though. :(
> + return connector_status_disconnected;
> +
> + return connector_status_connected;
> +}
> +
> +static struct drm_connector_funcs sun4i_hdmi_connector_funcs = {
> + .dpms = drm_atomic_helper_connector_dpms,
> + .detect = sun4i_hdmi_connector_detect,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .reset = drm_atomic_helper_connector_reset,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int sun4i_hdmi_bind(struct device *dev, struct device *master,
> + void *data)
> +{
> + struct drm_device *drm = data;
> + struct sun4i_drv *drv = drm->dev_private;
> + struct sun4i_hdmi *hdmi = dev_get_drvdata(dev);
> + int ret;
> +
> + hdmi->drv = drv;
> + drm_encoder_helper_add(&hdmi->encoder,
> + &sun4i_hdmi_helper_funcs);
> + ret = drm_encoder_init(drm,
> + &hdmi->encoder,
> + &sun4i_hdmi_funcs,
> + DRM_MODE_ENCODER_TMDS,
> + NULL);
> + if (ret) {
> + dev_err(dev, "Couldn't initialise the HDMI encoder\n");
> + return ret;
> + }
> +
> + hdmi->encoder.possible_crtcs = BIT(0);
You can use drm_of_find_possible_crtcs() now. See the TV encoder driver.
> +
> + drm_connector_helper_add(&hdmi->connector,
> + &sun4i_hdmi_connector_helper_funcs);
> + ret = drm_connector_init(drm, &hdmi->connector,
> + &sun4i_hdmi_connector_funcs,
> + DRM_MODE_CONNECTOR_HDMIA);
> + if (ret) {
> + dev_err(dev,
> + "Couldn't initialise the Composite connector\n");
Wrong connector.
> + goto err_cleanup_connector;
> + }
> + hdmi->connector.interlace_allowed = true;
> +
> + drm_mode_connector_attach_encoder(&hdmi->connector, &hdmi->encoder);
> +
> + return 0;
> +
> +err_cleanup_connector:
> + drm_encoder_cleanup(&hdmi->encoder);
> + return ret;
> +}
> +
> +static void sun4i_hdmi_unbind(struct device *dev, struct device *master,
> + void *data)
> +{
> + struct sun4i_hdmi *hdmi = dev_get_drvdata(dev);
> +
> + drm_connector_cleanup(&hdmi->connector);
> + drm_encoder_cleanup(&hdmi->encoder);
> +}
> +
> +static struct component_ops sun4i_hdmi_ops = {
> + .bind = sun4i_hdmi_bind,
> + .unbind = sun4i_hdmi_unbind,
> +};
> +
> +static int sun4i_hdmi_probe(struct platform_device *pdev)
> +{
> + struct sun4i_hdmi *hdmi;
> + struct resource *res;
> + int ret;
> +
> + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
> + if (!hdmi)
> + return -ENOMEM;
> + dev_set_drvdata(&pdev->dev, hdmi);
> + hdmi->dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + hdmi->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(hdmi->base)) {
> + dev_err(&pdev->dev, "Couldn't map the HDMI encoder registers\n");
> + return PTR_ERR(hdmi->base);
> + }
> +
> + hdmi->bus_clk = devm_clk_get(&pdev->dev, "ahb");
> + if (IS_ERR(hdmi->bus_clk)) {
> + dev_err(&pdev->dev, "Couldn't get the HDMI bus clock\n");
> + return PTR_ERR(hdmi->bus_clk);
> + }
> + clk_prepare_enable(hdmi->bus_clk);
> +
> + hdmi->mod_clk = devm_clk_get(&pdev->dev, "mod");
> + if (IS_ERR(hdmi->mod_clk)) {
> + dev_err(&pdev->dev, "Couldn't get the HDMI mod clock\n");
> + return PTR_ERR(hdmi->mod_clk);
> + }
> + clk_prepare_enable(hdmi->mod_clk);
> +
> + hdmi->pll0_clk = devm_clk_get(&pdev->dev, "pll-0");
> + if (IS_ERR(hdmi->pll0_clk)) {
> + dev_err(&pdev->dev, "Couldn't get the HDMI PLL 0 clock\n");
> + return PTR_ERR(hdmi->pll0_clk);
> + }
> +
> + hdmi->pll1_clk = devm_clk_get(&pdev->dev, "pll-1");
> + if (IS_ERR(hdmi->pll1_clk)) {
> + dev_err(&pdev->dev, "Couldn't get the HDMI PLL 1 clock\n");
> + return PTR_ERR(hdmi->pll1_clk);
> + }
> +
> + ret = sun4i_tmds_create(hdmi);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't create the TMDS clock\n");
> + return ret;
> + }
> +
> + writel(SUN4I_HDMI_CTRL_ENABLE, hdmi->base + SUN4I_HDMI_CTRL_REG);
> +
> +#define SUN4I_HDMI_PAD_CTRL0 0xfe800000
> +
> + writel(SUN4I_HDMI_PAD_CTRL0, hdmi->base + SUN4I_HDMI_PAD_CTRL0_REG);
> +
> + /* TODO: defines */
> + writel((6 << 3) | (2 << 10) | BIT(14) | BIT(15) |
> + BIT(19) | BIT(20) | BIT(22) | BIT(23),
> + hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> +
> + /* TODO: defines */
> + writel((8 << 0) | (7 << 8) | (239 << 12) | (7 << 17) | (4 << 20) |
> + BIT(25) | BIT(27) | BIT(28) | BIT(29) | BIT(30) | BIT(31),
> + hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
FYI some bits in this register look a lot like the MIPI PLL on the A33.
Bit 31 looks like the enable bit.
> +
> + ret = sun4i_ddc_create(hdmi, hdmi->tmds_clk);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't create the DDC clock\n");
> + return ret;
> + }
We do all this in the bind function for all the other components.
Any particular reason to do it differently here?
> +
> + return component_add(&pdev->dev, &sun4i_hdmi_ops);
> +}
> +
> +static int sun4i_hdmi_remove(struct platform_device *pdev)
> +{
> + component_del(&pdev->dev, &sun4i_hdmi_ops);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sun4i_hdmi_of_table[] = {
> + { .compatible = "allwinner,sun5i-a10s-hdmi" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_hdmi_of_table);
> +
> +static struct platform_driver sun4i_hdmi_driver = {
> + .probe = sun4i_hdmi_probe,
> + .remove = sun4i_hdmi_remove,
> + .driver = {
> + .name = "sun4i-hdmi",
> + .of_match_table = sun4i_hdmi_of_table,
> + },
> +};
> +module_platform_driver(sun4i_hdmi_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Allwinner A10 HDMI Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
> new file mode 100644
> index 000000000000..40f48f1d4685
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
> @@ -0,0 +1,236 @@
> +/*
> + * Copyright (C) 2016 Free Electrons
> + * Copyright (C) 2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/clk-provider.h>
> +
> +#include "sun4i_tcon.h"
> +#include "sun4i_hdmi.h"
> +
> +struct sun4i_tmds {
> + struct clk_hw hw;
> + struct sun4i_hdmi *hdmi;
> +};
> +
> +static inline struct sun4i_tmds *hw_to_tmds(struct clk_hw *hw)
> +{
> + return container_of(hw, struct sun4i_tmds, hw);
> +}
> +
> +
> +static unsigned long sun4i_tmds_calc_divider(unsigned long rate,
> + unsigned long parent_rate,
> + u8 *div,
> + bool *half)
> +{
> + unsigned long best_rate = 0;
> + u8 best_m = 0, m;
> + bool is_double;
> +
> + for (m = 1; m < 16; m++) {
> + u8 d;
> +
> + for (d = 1; d < 3; d++) {
> + unsigned long tmp_rate;
> +
> + tmp_rate = parent_rate / m / d;
> +
> + if (tmp_rate > rate)
> + continue;
> +
> + if (!best_rate ||
> + (rate - tmp_rate) < (rate - best_rate)) {
> + best_rate = tmp_rate;
> + best_m = m;
> + is_double = d;
> + }
> + }
> + }
> +
> + if (div && half) {
> + *div = best_m;
> + *half = is_double;
> + }
> +
> + return best_rate;
> +}
> +
> +
> +static int sun4i_tmds_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_hw *parent;
> + unsigned long best_parent = 0;
> + unsigned long rate = req->rate;
> + int best_div = 1, best_half = 1;
> + int i, j;
> +
> + printk("%s %d rate %lu\n", __func__, __LINE__, rate);
Stray printk?
> +
> + /*
> + * We only consider PLL3, since the TCON is very likely to be
> + * clocked from it, and to have the same rate than our HDMI
> + * clock, so we should not need to do anything.
> + */
> +
> + parent = clk_hw_get_parent_by_index(hw, 0);
> + if (!parent)
> + return -EINVAL;
> +
> + for (i = 1; i < 3; i++) {
> + for (j = 1; j < 16; j++) {
> + unsigned long ideal = rate * i * j;
> + unsigned long rounded;
> +
> + rounded = clk_hw_round_rate(parent, ideal);
> +
> + if (rounded == ideal) {
> + best_parent = rounded;
> + best_half = i;
> + best_div = j;
> + goto out;
> + }
> +
> + if (abs(rate - rounded / i) <
> + abs(rate - best_parent / best_div)) {
> + best_parent = rounded;
> + best_div = i;
> + }
> + }
> + }
> +
> +out:
> + req->rate = best_parent / best_half / best_div;
> + req->best_parent_rate = best_parent;
> + req->best_parent_hw = parent;
> +
> + printk("%s %d rate %lu parent rate %lu (%s) div %d half %d\n",
> + __func__, __LINE__, req->rate, req->best_parent_rate,
> + clk_hw_get_name(req->best_parent_hw),
> + best_div, best_half);
Stray printk?
> +
> + return 0;
> +}
> +
> +static unsigned long sun4i_tmds_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct sun4i_tmds *tmds = hw_to_tmds(hw);
> + u32 reg;
> +
> + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> + if (reg & SUN4I_HDMI_PAD_CTRL1_HALVE_CLK)
> + parent_rate /= 2;
> +
> + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
> + reg = (reg >> 4) & 0xf;
> + if (!reg)
> + reg = 1;
> +
> + return parent_rate / reg;
> +}
> +
> +static int sun4i_tmds_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct sun4i_tmds *tmds = hw_to_tmds(hw);
> + bool half;
> + u32 reg;
> + u8 div;
> +
> + sun4i_tmds_calc_divider(rate, parent_rate, &div, &half);
> +
> + printk("%s %d rate %lu parent rate %lu div %d half %s\n",
> + __func__, __LINE__, rate, parent_rate, div,
> + half ? "yes" : "no");
Stray printk?
> +
> + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> + reg &= ~SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
> + if (half)
> + reg |= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;
> + writel(reg, tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);
> +
> + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
> + reg &= ~SUN4I_HDMI_PLL_CTRL_DIV_MASK;
> + writel(reg | SUN4I_HDMI_PLL_CTRL_DIV(div),
> + tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
> +
> + return 0;
> +}
> +
> +static u8 sun4i_tmds_get_parent(struct clk_hw *hw)
> +{
> + struct sun4i_tmds *tmds = hw_to_tmds(hw);
> + u32 reg;
> +
> + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);
> + return ((reg & SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK) >>
> + SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT);
> +}
> +
> +static int sun4i_tmds_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct sun4i_tmds *tmds = hw_to_tmds(hw);
> + u32 reg;
> +
> + if (index > 1)
> + return -EINVAL;
> +
> + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);
> + reg &= ~SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK;
> + writel(reg | SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(index),
> + tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops sun4i_tmds_ops = {
> + .determine_rate = sun4i_tmds_determine_rate,
> + .recalc_rate = sun4i_tmds_recalc_rate,
> + .set_rate = sun4i_tmds_set_rate,
> +
> + .get_parent = sun4i_tmds_get_parent,
> + .set_parent = sun4i_tmds_set_parent,
> +};
> +
> +int sun4i_tmds_create(struct sun4i_hdmi *hdmi)
> +{
> + struct clk_init_data init;
> + struct sun4i_tmds *tmds;
> + const char *parents[2];
> +
> + parents[0] = __clk_get_name(hdmi->pll0_clk);
> + if (!parents[0])
> + return -ENODEV;
> +
> + parents[1] = __clk_get_name(hdmi->pll1_clk);
> + if (!parents[1])
> + return -ENODEV;
> +
> + tmds = devm_kzalloc(hdmi->dev, sizeof(*tmds), GFP_KERNEL);
> + if (!tmds)
> + return -ENOMEM;
> +
> + init.name = "hdmi-tmds";
> + init.ops = &sun4i_tmds_ops;
> + init.parent_names = parents;
> + init.num_parents = 2;
> + init.flags = CLK_SET_RATE_PARENT;
> +
> + tmds->hdmi = hdmi;
> + tmds->hw.init = &init;
> +
> + hdmi->tmds_clk = devm_clk_register(hdmi->dev, &tmds->hw);
> + if (IS_ERR(hdmi->tmds_clk))
> + return PTR_ERR(hdmi->tmds_clk);
> +
> + return 0;
> +}
> --
I also compared the manuals of A20 and A31, and the existing U-boot driver.
So far it looks like the DDC bits are quite different. We could probably
use regfields to work around it, but the DDC clock formula is completely
different. The TMDS clock pre-divider is also different, your usual sun4i
vs sun6i factor offset. Last, the initial values for the 3 PLL related
registers are different.
I'm currently working on an A31 variant for this, basically just copying
the DDC and TMDS bits.
Regards
ChenYu
> git-series 0.8.11
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
^ 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