Re: [PATCH 19/32] fbdev/ps3fb: Initialize fb_ops with fbdev macros

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Initialize the instance of struct fb_ops with fbdev initializer
> macros for framebuffers in virtual address space. Set the read/write,
> draw and mmap callbacks to the correct implementation and avoid
> implicit defaults. Also select the necessary helpers in Kconfig.
>
> Fbdev drivers sometimes rely on the callbacks being NULL for a
> default I/O-memory-based implementation to be invoked; hence
> requiring the I/O helpers to be built in any case. Setting all
> callbacks in all drivers explicitly will allow to make the I/O
> helpers optional. This benefits systems that do not use these
> functions.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Michael Ellerman 
> Cc: Nicholas Piggin 
> Cc: Christophe Leroy 
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  drivers/video/fbdev/Kconfig | 5 +
>  drivers/video/fbdev/ps3fb.c | 7 ++-
>  2 files changed, 3 insertions(+), 9 deletions(-)
>

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 18/32] fbdev/ps3fb: Set FBINFO_VIRTFB flag

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> The ps3fb driver operates on system memory. Mark the framebuffer
> accordingly. Helpers operating on the framebuffer memory will test
> for the presence of this flag.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Michael Ellerman 
> Cc: Nicholas Piggin 
> Cc: Christophe Leroy 
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  drivers/video/fbdev/ps3fb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v5 2/5] fbdev: Replace fb_pgprotect() with pgprot_framebuffer()

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Rename the fbdev mmap helper fb_pgprotect() to pgprot_framebuffer().
> The helper sets VMA page-access flags for framebuffers in device I/O
> memory.
>
> Also clean up the helper's parameters and return value. Instead of
> the VMA instance, pass the individial parameters separately: existing
> page-access flags, the VMAs start and end addresses and the offset
> in the underlying device memory rsp file. Return the new page-access
> flags. These changes align pgprot_framebuffer() with other pgprot_()
> functions.
>
> v4:
>   * fix commit message (Christophe)
> v3:
>   * rename fb_pgprotect() to pgprot_framebuffer() (Arnd)
>
> Signed-off-by: Thomas Zimmermann 
> Reviewed-by: Arnd Bergmann 
> Acked-by: Geert Uytterhoeven  # m68k
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v4 2/5] fbdev: Replace fb_pgprotect() with pgprot_framebuffer()

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

Hello Thomas,

> Rename the fbdev mmap helper fb_pgprotect() to pgprot_framebuffer().
> The helper sets VMA page-access flags for framebuffers in device I/O
> memory.
>

I think this rename makes it more clear.

> Also clean up the helper's parameters and return value. Instead of
> the VMA instance, pass the individial parameters separately: existing
> page-access flags, the VMAs start and end addresses and the offset

But I fail to see the benefit of this part. Could you please add an
explanation about why this change is desirable ?

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v4 1/5] fbdev: Avoid file argument in fb_pgprotect()

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

Hello Thomas,

> Only PowerPC's fb_pgprotect() needs the file argument, although
> the implementation does not use it. Pass NULL to the internal

Can you please mention the function that's the implementation for
PowerPC ? If I'm looking at the code correctly, that function is
phys_mem_access_prot() defined in the arch/powerpc/mm/mem.c file:

pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
  unsigned long size, pgprot_t vma_prot)
{
if (ppc_md.phys_mem_access_prot)
return ppc_md.phys_mem_access_prot(file, pfn, size, vma_prot);

if (!page_is_ram(pfn))
vma_prot = pgprot_noncached(vma_prot);

return vma_prot;
}

and if set, ppc_md.phys_mem_access_prot is pci_phys_mem_access_prot()
that is defined in the arch/powerpc/kernel/pci-common.c source file:

https://elixir.bootlin.com/linux/v6.6-rc2/source/arch/powerpc/kernel/pci-common.c#L524

That function indeed doesn't use the file argument. So your patch looks
correct to me.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 9/9] efi: move screen_info into efi init code

[Javier Martinez Canillas]

Arnd Bergmann  writes:

> From: Arnd Bergmann 
>
> After the vga console no longer relies on global screen_info, there are
> only two remaining use cases:
>
>  - on the x86 architecture, it is used for multiple boot methods
>(bzImage, EFI, Xen, kexec) to commicate the initial VGA or framebuffer

communicate

>settings to a number of device drivers.
>
>  - on other architectures, it is only used as part of the EFI stub,
>and only for the three sysfb framebuffers (simpledrm, simplefb, efifb).
>
> Remove the duplicate data structure definitions by moving it into the
> efi-init.c file that sets it up initially for the EFI case, leaving x86
> as an exception that retains its own definition for non-EFI boots.
>
> The added #ifdefs here are optional, I added them to further limit the
> reach of screen_info to configurations that have at least one of the
> users enabled.
>
> Signed-off-by: Arnd Bergmann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 8/9] hyperv: avoid dependency on screen_info

[Javier Martinez Canillas]

Arnd Bergmann  writes:

> From: Arnd Bergmann 
>
> The two hyperv framebuffer drivers (hyperv_fb or hyperv_drm_drv) access the
> global screen_info in order to take over from the sysfb framebuffer, which
> in turn could be handled by simplefb, simpledrm or efifb. Similarly, the
> vmbus_drv code marks the original EFI framebuffer as reserved, but this
> is not required if there is no sysfb.
>
> As a preparation for making screen_info itself more local to the sysfb
> helper code, add a compile-time conditional in all three files that relate
> to hyperv fb and just skip this code if there is no sysfb that needs to
> be unregistered.
>
> Signed-off-by: Arnd Bergmann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 7/9] vga16fb: drop powerpc support

[Javier Martinez Canillas]

Arnd Bergmann  writes:

> From: Arnd Bergmann 
>
> I noticed that commit 0db5b61e0dc07 ("fbdev/vga16fb: Create
> EGA/VGA devices in sysfb code") broke vga16fb on non-x86 platforms,
> because the sysfb code never creates a vga-framebuffer device when
> screen_info.orig_video_isVGA is set to '1' instead of VIDEO_TYPE_VGAC.
>
> However, it turns out that the only architecture that has allowed
> building vga16fb in the past 20 years is powerpc, and this only worked
> on two 32-bit platforms and never on 64-bit powerpc. The last machine
> that actually used this was removed in linux-3.10, so this is all dead
> code and can be removed.
>
> The big-endian support in vga16fb.c could also be removed, but I'd just
> leave this in place.
>
> Fixes: 933ee7119fb14 ("powerpc: remove PReP platform")
> Signed-off-by: Arnd Bergmann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 6/9] vgacon: clean up global screen_info instances

[Javier Martinez Canillas]

Arnd Bergmann  writes:

> From: Arnd Bergmann 
>
> To prepare for completely separating the VGA console screen_info from
> the one used in EFI/sysfb, rename the vgacon instances and make them
> local as much as possible.
>
> ia64 and arm both have confurations with vgacon and efi, but the contents

is this a typo for configurations ?

> never overlaps because ia64 has no EFI framebuffer, and arm only has
> vga console on legacy platforms without EFI. Renaming these is required
> before the EFI screen_info can be moved into drivers/firmware.
>
> The ia64 vga console is actually registered in two places from
> setup_arch(), but one of them is wrong, so drop the one in pcdp.c and
> the fix the one in setup.c to use the correct conditional.
>

s/the fix the/fix the

> x86 has to keep them together, as the boot protocol is used to switch
> between VGA text console and framebuffer through the screen_info data.
>
> Signed-off-by: Arnd Bergmann 
> ---

Patch looks good to me, but I'm not that familiar with some of the arches
to give a proper reviewed-by.

Acked-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 5/9] vgacon: remove screen_info dependency

[Javier Martinez Canillas]

Arnd Bergmann  writes:

> From: Arnd Bergmann 
>
> The vga console driver is fairly self-contained, and only used by
> architectures that explicitly initialize the screen_info settings.
>
> Chance every instance that picks the vga console by setting conswitchp
> to call a function instead, and pass a reference to the screen_info
> there.
>
> Signed-off-by: Arnd Bergmann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/4] vgacon: rework screen_info #ifdef checks

[Javier Martinez Canillas]

"Arnd Bergmann"  writes:

> On Fri, Jul 7, 2023, at 15:40, Javier Martinez Canillas wrote:

[...]

>> And this is only used by mdacon (not supported by ia64), vgacon and
>> vga16fb (not supported by ia64 either).
>>
>> So this could just be guarded just by CONFIG_VGA_CONSOLE for ia64 ?
>
> Right, I though about doing this more accurately, but in the end
> went for the simplest change rather than spending much more time
> trying to clean up the unused variables etc.
>
> Let me know if you'd prefer me to respin this part, otherwise
> I'd call the ia64 bit good enough for the purpose of the series.
>

No need to re-spin, agreed that makes sense to keep it simpler.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/4] vgacon: rework screen_info #ifdef checks

[Javier Martinez Canillas]

Arnd Bergmann  writes:

> From: Arnd Bergmann 
>
> On non-x86 architectures, the screen_info variable is generally only
> used for the VGA console where supported, and in some cases the EFI
> framebuffer or vga16fb.
>
> Now that we have a definite list of which architectures actually use it
> for what, use consistent #ifdef checks so the global variable is only
> defined when it is actually used on those architectures.
>
> On powerpc, there is no support for vgacon, but there is support for
> vga16fb. Loongarch and riscv have no support for vgacon or vga16fb, but
> they support EFI firmware, so only that needs to be checked, and the
> initialization can be removed because that is handled by EFI.
> IA64 has both vgacon and EFI.
>
> Signed-off-by: Arnd Bergmann 
> ---

[...]

> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 5a55ac82c13a4..0c09ff7fde46b 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -86,9 +86,11 @@ EXPORT_SYMBOL(local_per_cpu_offset);
>  #endif
>  unsigned long ia64_cycles_per_usec;
>  struct ia64_boot_param *ia64_boot_param;
> +#if defined(CONFIG_VGA_CONSOLE) || defined(CONFIG_EFI)
>  struct screen_info screen_info;

I think that only screen_info should be guarded by both symbols ?

>  unsigned long vga_console_iobase;

It seems this variable was never used since it was introduced by commit
66b7f8a30437 ("[IA64-SGI] pcdp: add PCDP pci interface support") ?

>  unsigned long vga_console_membase;

And this is only used by mdacon (not supported by ia64), vgacon and
vga16fb (not supported by ia64 either).

So this could just be guarded just by CONFIG_VGA_CONSOLE for ia64 ?

The rest of the patch looks good to me.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 10/12] fbdev/core: Use fb_is_primary_device() in fb_firmware_edid()

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Detect the primary VGA device with fb_is_primary_device() in the
> implementation of fb_firmware_edid(). Remove the existing code.
>

An explanation about why this is possible would be useful here.

> Adapt the function to receive an instance of struct fb_info and
> update all callers.
>

[...]

> -const unsigned char *fb_firmware_edid(struct device *device)
> +const unsigned char *fb_firmware_edid(struct fb_info *info)
>  {
> - struct pci_dev *dev = NULL;
> - struct resource *res = NULL;
>   unsigned char *edid = NULL;
>  
> - if (device)
> - dev = to_pci_dev(device);
> -
> - if (dev)
> - res = >resource[PCI_ROM_RESOURCE];
> -
> - if (res && res->flags & IORESOURCE_ROM_SHADOW)

This open codes what used to be the fb_is_primary_device() logic before
commit 5ca1479cd35d ("fbdev: Simplify fb_is_primary_device for x86").
But now after that commit there is functional change since the ROM
shadowing check would be dropped.

I believe that's OK and Sima explains in their commit message that
vga_default_device() should be enough and the check is redundant.

Still, I think that this change should be documented in your commit
message. 

With that change,

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 08/12] drivers/firmware: Remove trailing whitespaces

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Fix coding style. No functional changes.
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 


-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 05/12] arch: Remove trailing whitespaces

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Fix coding style. No functional changes.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Yoshinori Sato 
> Cc: Rich Felker 
> Cc: John Paul Adrian Glaubitz 
> Cc: Andrew Morton 
> Cc: Geert Uytterhoeven 
> Cc: Arnd Bergmann 
> Cc: "Kirill A. Shutemov" 
> Cc: Anshuman Khandual 
> Cc: Niklas Schnelle 
> Cc: Zi Yan 
> Cc: "Mike Rapoport (IBM)" 
> Cc: Peter Zijlstra 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 04/12] staging/sm750fb: Do not include

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> The sm750fb driver does not need anything from .
> Remove the include statements.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Sudip Mukherjee 
> Cc: Teddy Wang 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 03/12] sysfb: Do not include from sysfb header

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> The header file  does not need anything from
> . Declare struct screen_info and remove
> the include statements.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Ard Biesheuvel 
> Cc: Hans de Goede 
> Cc: Javier Martinez Canillas 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 02/12] fbdev/sm712fb: Do not include

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Sm712fb's dependency on  is artificial in that
> it only uses struct screen_info for its internals. Replace the use of
> struct screen_info with a custom data structure and remove the include
> of .
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Sudip Mukherjee 
> Cc: Teddy Wang 
> Cc: Helge Deller 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 01/12] efi: Do not include from EFI header

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> The header file  does not need anything from
> . Declare struct screen_info and remove
> the include statements. Update a number of source files that
> require struct screen_info's definition.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Ard Biesheuvel 
> Cc: Russell King 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 02/11] fbdev: Transfer video= option strings to caller; clarify ownership

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Hi
>
> Am 17.02.23 um 09:37 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann  writes:
>> 
>>> In fb_get_options(), always duplicate the returned option string and
>>> transfer ownership of the memory to the function's caller.
>>>
>>> Until now, only the global option string got duplicated and transferred
>>> to the caller; the per-driver options were owned by fb_get_options().
>>> In the end, it was impossible for the function's caller to detect if
>>> it had to release the string's memory buffer. Hence, all calling drivers
>>> leak the memory buffer. The leaks have existed ever since, but drivers
>>> only call fb_get_option() once as part of module initialization. So the
>>> amount of leaked memory is not significant.
>>>
>>> Fix the semantics of fb_get_option() by unconditionally transferring
>>> ownership of the memory buffer to the caller. Later patches can resolve
>>> the memory leaks in the fbdev drivers.
>>>
>>> Signed-off-by: Thomas Zimmermann 
>>> ---
>> 
>> [...]
>> 
>>> +   if (option) {
>>> +   if (options)
>>> +   *option = kstrdup(options, GFP_KERNEL);
>>> +   else
>>> +   *option = NULL;
>>> +   }
>>>
>> 
>> I know the old code wasn't checking if kstrdup() succeeded, but you should
>
> Kstrdup uses kmalloc, which already warns about failed allocations. I 
> think it's discouraged to warn again. (Wasn't there a warning in sparse 
> or checkpatch?)  So I'd rather leave it as is.
>

I didn't mean to warn but to return an error code.

>> do it here and let the caller know. And same if (!options). So I guess the
>> following check can be added (to be consistent with the rest of the code):
>> 
>>  if (!*option)
>>  retval = 1;
>
> Why is that needed for consistency?
>
> Retval is the state of the output: enabled or not. If there are no 
> options, retval should be 0(=enabled). 1(=disabled) is only set by 
> video=off or that ofonly thing.
>

Ah, I see. I misundertood what retval was about. Forget this comment then.

Maybe while you are there could have another patch to document the return
value in the fb_get_options() kernel-doc?

And this patch looks good to me too after your explanations.

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 11/11] drm: Fix comment on mode parsing

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Do not claim that there's a default mode in the video= option parser.
> if no option string has been given, the parser does nothing.
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 10/11] drm: Include for mode parsing

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Include  in drm_connector.c to get video_get_options()
> and avoid the dependency on . The replaced function
> fb_get_options() is just a tiny wrapper around video_get_opions(). No
> functional changes.
>
> Include  to get fwnode_handle_put(), which had been
> provided via .
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 09/11] driver/ps3: Include for mode parsing

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Include  in ps3av.c to get video_get_options() and
> avoid the dependency on . The replaced function
> fb_get_options() is just a tiny wrapper around video_get_opions(). No
> functional changes.
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 08/11] fbdev: Handle video= parameter in video/cmdline.c

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Handle the command-line parameter video= in video/cmdline.c. Implement
> the fbdev helper fb_get_options() on top. Will allows to handle the
> kernel parameter in DRM without fbdev dependencies.
>
> Note that __video_get_options() has the meaning of its return value
> inverted compared to fb_get_options(). The new helper returns true if
> the adapter has been enabled, and false otherwise.
>
> There is the ofonly parameter, which disables output for non-OF-based
> framebuffers. It is only for offb and looks like a workaround. The actual
> purpose it not clear to me. Use 'video=off' or 'nomodeset' instead.
>

s/it/is

> Signed-off-by: Thomas Zimmermann 
> ---

[..]

> +#include  /* for FB_MAX */

I believe including  is enough here.

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 07/11] fbdev: Move option-string lookup into helper

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Move the lookup of the option string into an internal helper. No
> functional changes.
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 06/11] fbdev: Unexport fb_mode_option

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> There are no external users of fb_mode_option. Unexport the variable
> and declare it static.
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 05/11] fbdev: Read video= option with fb_get_option() in modedb

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Get the kernel's global video= parameter with fb_get_option(). Done
> to unexport the internal fbdev state fb_mode_config. No functional
> changes.
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 04/11] drivers/ps3: Read video= option with fb_get_option()

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Get the kernel's global video= parameter with fb_get_option(). Done
> to unexport the internal fbdev state fb_mode_config. No functional
> changes.
>
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 03/11] fbdev: Support NULL for name in option-string lookup

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Ignore the per-driver video options if no driver name has been
> specified to fb_get_option(). Return the global options in this
> case.
>
> Signed-off-by: Thomas Zimmermann 
> ---

I think you need to update the kernel-doc as well to mention that
@name could be NULL ?

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH 02/11] fbdev: Transfer video= option strings to caller; clarify ownership

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> In fb_get_options(), always duplicate the returned option string and
> transfer ownership of the memory to the function's caller.
>
> Until now, only the global option string got duplicated and transferred
> to the caller; the per-driver options were owned by fb_get_options().
> In the end, it was impossible for the function's caller to detect if
> it had to release the string's memory buffer. Hence, all calling drivers
> leak the memory buffer. The leaks have existed ever since, but drivers
> only call fb_get_option() once as part of module initialization. So the
> amount of leaked memory is not significant.
>
> Fix the semantics of fb_get_option() by unconditionally transferring
> ownership of the memory buffer to the caller. Later patches can resolve
> the memory leaks in the fbdev drivers.
>
> Signed-off-by: Thomas Zimmermann 
> ---

[...]

> + if (option) {
> + if (options)
> + *option = kstrdup(options, GFP_KERNEL);
> + else
> + *option = NULL;
> + }
>

I know the old code wasn't checking if kstrdup() succeeded, but you should
do it here and let the caller know. And same if (!options). So I guess the
following check can be added (to be consistent with the rest of the code):

if (!*option)
retval = 1;

>   return retval;
>  }
> -- 
> 2.39.1

Best regards,
Javier

Re: [PATCH 01/11] fbdev: Fix contact info in fb_cmdline.c

[Javier Martinez Canillas]

Thomas Zimmermann  writes:

> Fix Daniel's email address. No functional changes.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Daniel Vetter 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier

Re: [PATCH v4 5/5] drm/ofdrm: Support big-endian scanout buffers

[Javier Martinez Canillas]

Hello,

On 10/12/22 16:27, Michal Suchánek wrote:

[...]

>>
>> If you are using the framebuffer code from vga.c, I would guess that
>> that you can run a big-endian kernel with qemu-system-ppc64,
>> or a little-endian kernel with qemu-system-ppc64le and get the
>> correct colors, while running a little-endian kernel with
>> qemu-system-ppc64 and vga.c, or using a different framebuffer
>> emulation on a big-endian kernel would give you the wrong colors.
> 
> Thanks for digging this up.
> 
> That makes one thing clear: qemu does not emulate this framebuffer
> property correctly, and cannot be relied on for verification.
> 
> If you can provide test results from real hardware that show the current
> logic as flawed it should be changed.
> 
> In absence of such test results I think the most reasonable thing is to
> keep the logic that nobody complained about for 10+ years.
> 

I agree with Michal and Thomas on this. I don't see a strong reason to not
use the same heuristic that the offb fbdev driver has been doing for this.

Otherwise if this turns out to be needed, it will cause a regression for a
user that switches to this driver instead. Specially since both fbdev and
DRM drivers match against the same "display" OF compatible string.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v4 5/5] drm/ofdrm: Support big-endian scanout buffers

[Javier Martinez Canillas]

Hello Thomas,

On 9/28/22 12:50, Thomas Zimmermann wrote:
> All DRM formats assume little-endian byte order. On big-endian systems,
> it is likely that the scanout buffer is in big endian as well. Update

You say it is likely, not always then? Does it depend on whether the Open
Firmware is BE or LE ?

[...]

> +static bool display_get_big_endian_of(struct drm_device *dev, struct 
> device_node *of_node)
> +{
> + bool big_endian;
> +
> +#ifdef __BIG_ENDIAN
> + big_endian = true;
> + if (of_get_property(of_node, "little-endian", NULL))
> + big_endian = false;
> +#else
> + big_endian = false;
> + if (of_get_property(of_node, "big-endian", NULL))
> + big_endian = true;
> +#endif
> +
> + return big_endian;
> +}
> +

Ah, I see. The heuristic then is whether the build is BE or LE or if the Device
Tree has an explicit node defining the endianess. The patch looks good to me:

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 10/10] drm/ofdrm: Support color management

[Javier Martinez Canillas]

On 7/27/22 10:41, Thomas Zimmermann wrote:

[...]

>>
>>> +static void __iomem *ofdrm_mach64_cmap_ioremap(struct ofdrm_device *odev,
>>> +  struct device_node *of_node,
>>> +  u64 fb_base)
>>> +{
>>> +   struct drm_device *dev = >dev;
>>> +   u64 address;
>>> +   void __iomem *cmap_base;
>>> +
>>> +   address = fb_base & 0xff00ul;
>>> +   address += 0x7ff000;
>>> +
>>
>> It would be good to know where these addresses are coming from. Maybe some
>> constant macros or a comment ? Same for the other places where addresses
>> and offsets are used.
> 
> I have no idea where these values come from. I took them from offb. And 
> I suspect that some of these CMAP helpers could be further merged if 
> only it was clear where the numbers come from.  But as i don't have the 
> equipment for testing, I took most of this literally as-is from offb.
>

I see. As Michal mentioned maybe someone more familiar with this platform
could shed some light about these but in any case that could be done later.

[...]

>>> +
>>> +   new_crtc_state = drm_atomic_get_new_crtc_state(new_state, 
>>> new_plane_state->crtc);
>>> +
>>> +   new_ofdrm_crtc_state = to_ofdrm_crtc_state(new_crtc_state);
>>> +   new_ofdrm_crtc_state->format = new_fb->format;
>>> +
>>
>> Ah, I understand now why you didn't factor out the .atomic_check callbacks
>> for the two drivers in a fwfb helper. Maybe you can also add a comment to
>> mention that this updates the format so the CRTC palette can be applied in
>> the .atomic_flush callback ?
> 
> Yeah, this code is one reason for not sharing atomic_check in fwfb.  The 
> other reason is that the fwfb code is only a wrapper around the atomic 
> helpers with little extra value.  I did have such fwfb helpers a some 
> point, but removed them.
>

Got it.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 06/10] drm/simpledrm: Move some functionality into fwfb helper library

[Javier Martinez Canillas]

On 7/27/22 10:24, Thomas Zimmermann wrote:
> Hi
> 
> Am 25.07.22 um 18:23 schrieb Javier Martinez Canillas:
>> On 7/20/22 16:27, Thomas Zimmermann wrote:
>>> Move some of simpledrm's functionality into a helper library. Other
>>> drivers for firmware-provided framebuffers will also need functions
>>> to handle fixed modes and color formats, or update the back buffer.
>>>
>>> Signed-off-by: Thomas Zimmermann 
>>> ---
>>
>> Nice patch!
> 
> TBH it took me 3 tries to get something done for this library and I'm 
> still not happy with the result. I want to share code between simpledrm 
> and ofdrm, but that turns out to be harder then expected. A good part of 
> this code appears to belong into other libraries (you also mentioned 
> this below).
> 
> I don't want to duplicated code between simpledrm and ofdrm without 
> reason, but I expect that this library will somewhen be refactored and 
> dissolved into existing libraries.
>

Yes, I think is a step in the right direction and guess it would be even
more useful once/if a 3rd firmware-provided framebuffer driver is added.

> 
>>
>> [...]
>>
>>> +
>>> +/**
>>> + * DOC: overview
>>> + *
>>> + * The Firmware Framebuffer library FWFB provides helpers for devices with
>>> + * fixed-mode backing storage. It helps drivers to export a display mode of
>>> + * te correct size and copy updates to the backing storage.
>>
>> the
>>
>> it is "backing storage" or "backing store" ? I always thought that storage 
>> was
>> used for non-volatile media while "store" could be volatile and non-volatile.
> 
> Why store? Isn't that a little shop for fashion or groceries? I'm no 
> native speaker; I can't tell if either implies that we're sending 
> pictures to a warehouse or bakery. :)
> 

LOL.

> Would 'back buffer' (in contrast to 'shadow buffer') be clear?
>

Back buffer is more clear indeed.

[...]

>> It seems a little bit arbitrary to me that format is the only field that's
>> a pointer and the other ones are embedded into the struct drm_fwfb. Any
>> reason for that or is just a consequence of how types were used by the
>> simpledrm_device_create() function before that code moved into helpers ?
> 
> Format is constant and comes from statically initialized memory in 
> drm_fourcc.c. I'd expect to be able to compare formats by comparing the 
> pointers. Copying the format here would break the assumption.
>

I see. Makes sense.

>>
>> [...]
>>
>>> +static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, 
>>> uint32_t fourcc)
>>> +{
>>> +   const uint32_t *fourccs_end = fourccs + nfourccs;
>>> +
>>> +   while (fourccs < fourccs_end) {
>>> +   if (*fourccs == fourcc)
>>> +   return true;
>>> +   ++fourccs;
>>> +   }
>>> +   return false;
>>> +}
>>
>> This seems a helper that could be useful besides the drm_fwfb_helper.c file.
>>
>> I believe patches 1-6 shouldn't wait for the others in this series and could
>> just be merged when ready. Patches 7-10 can follow later.
> 
> Yeah, I'd like to move patches 1 to 5 into a new series for merging. 
> Patch 6 is only useful for ofdrm and as I said, maybe there's a better 
> solution then this library. I'd rather keep it here for now.
>

OK.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 02/10] drm/simpledrm: Inline device-init helpers

[Javier Martinez Canillas]

Hello Thomas,

On 7/27/22 09:50, Thomas Zimmermann wrote:
> Hi
> 
> Am 25.07.22 um 17:01 schrieb Javier Martinez Canillas:
>> Hello Thomas,
>>
>> On 7/20/22 16:27, Thomas Zimmermann wrote:
>>> Inline the helpers for initializing the hardware FB, the memory
>>> management and the modesetting into the device-creation function.
>>> No functional changes.
>>>
>>
>> Could you please elaborate in the commit message why this change is
>> desirable?  Without this additional context, this feels like going
>> backwards, since you are dropping few helpers that have quite self
>> contained code and making simpledrm_device_create() much larger.
> 
> To clarify: I want to make the init code more easy to follow. These old 
> init functions still had to be called in the right order as each > possibly 
> depends on settings from the others. It also feels like it's 
> easier to extract common code for ofdrm. And the pipeline is static, so 
> it doesn't require complex chains of helper calls. Having everything in 
> one helper seems beneficial. (It's a trade-off, I know.)
>

I see. That makes sense to me. Could you please add the explanation to
the commit message ? And feel free to add my Acked-by for this one too.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 09/10] drm/ofdrm: Add per-model device function

[Javier Martinez Canillas]

Hello Michal,

On 7/26/22 16:40, Michal Suchánek wrote:
> Hello,
> 
> On Tue, Jul 26, 2022 at 03:38:37PM +0200, Javier Martinez Canillas wrote:
>> On 7/20/22 16:27, Thomas Zimmermann wrote:
>>> Add a per-model device-function structure in preparation of adding
>>> color-management support. Detection of the individual models has been
>>> taken from fbdev's offb.
>>>
>>> Signed-off-by: Thomas Zimmermann 
>>> ---
>>
>> Reviewed-by: Javier Martinez Canillas 
>>
>> [...]
>>
>>> +static bool is_avivo(__be32 vendor, __be32 device)
>>> +{
>>> +   /* This will match most R5xx */
>>> +   return (vendor == 0x1002) &&
>>> +  ((device >= 0x7100 && device < 0x7800) || (device >= 0x9400));
>>> +}
>>
>> Maybe add some constant macros to not have these magic numbers ?
> 
> This is based on the existing fbdev implementation's magic numbers:
> 
> drivers/video/fbdev/offb.c: ((*did >= 0x7100 && *did < 
> 0x7800) ||
>

Ah, I see. Then we might have to go with the magic numbers...
 
> Of course, it would be great if somebody knowledgeable could clarify
> those.
>

Indeed.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 10/10] drm/ofdrm: Support color management

[Javier Martinez Canillas]

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Support the CRTC's color-management property and implement each model's
> palette support.
> 
> The OF hardware has different methods of setting the palette. The
> respective code has been taken from fbdev's offb and refactored into
> per-model device functions. The device functions integrate this
> functionality into the overall modesetting.
> 
> As palette handling is a CRTC property that depends on the primary
> plane's color format, the plane's atomic_check helper now updates the
> format field in ofdrm's custom CRTC state. The CRTC's atomic_flush
> helper updates the palette for the format as needed.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

[...]

> +static void __iomem *ofdrm_mach64_cmap_ioremap(struct ofdrm_device *odev,
> +struct device_node *of_node,
> +u64 fb_base)
> +{
> + struct drm_device *dev = >dev;
> + u64 address;
> + void __iomem *cmap_base;
> +
> + address = fb_base & 0xff00ul;
> + address += 0x7ff000;
> +

It would be good to know where these addresses are coming from. Maybe some
constant macros or a comment ? Same for the other places where addresses
and offsets are used.

[...]

>  static struct ofdrm_crtc_state *to_ofdrm_crtc_state(struct drm_crtc_state 
> *base)
> @@ -376,10 +735,12 @@ static int 
> ofdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
>  struct drm_atomic_state 
> *new_state)
>  {
>   struct drm_plane_state *new_plane_state = 
> drm_atomic_get_new_plane_state(new_state, plane);
> + struct drm_framebuffer *new_fb = new_plane_state->fb;
>   struct drm_crtc_state *new_crtc_state;
> + struct ofdrm_crtc_state *new_ofdrm_crtc_state;
>   int ret;
>  
> - if (!new_plane_state->fb)
> + if (!new_fb)
>   return 0;
>  
>   new_crtc_state = drm_atomic_get_new_crtc_state(new_state, 
> new_plane_state->crtc);
> @@ -391,6 +752,14 @@ static int 
> ofdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   if (ret)
>   return ret;
>  
> + if (!new_plane_state->visible)
> + return 0;
> +
> + new_crtc_state = drm_atomic_get_new_crtc_state(new_state, 
> new_plane_state->crtc);
> +
> + new_ofdrm_crtc_state = to_ofdrm_crtc_state(new_crtc_state);
> + new_ofdrm_crtc_state->format = new_fb->format;
> +

Ah, I understand now why you didn't factor out the .atomic_check callbacks
for the two drivers in a fwfb helper. Maybe you can also add a comment to
mention that this updates the format so the CRTC palette can be applied in
the .atomic_flush callback ?

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 09/10] drm/ofdrm: Add per-model device function

[Javier Martinez Canillas]

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Add a per-model device-function structure in preparation of adding
> color-management support. Detection of the individual models has been
> taken from fbdev's offb.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

[...]

> +static bool is_avivo(__be32 vendor, __be32 device)
> +{
> + /* This will match most R5xx */
> + return (vendor == 0x1002) &&
> +((device >= 0x7100 && device < 0x7800) || (device >= 0x9400));
> +}

Maybe add some constant macros to not have these magic numbers ?

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 08/10] drm/ofdrm: Add CRTC state

[Javier Martinez Canillas]

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Add a dedicated CRTC state to ofdrm to later store information for
> palette updates.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/tiny/ofdrm.c | 62 ++--
>  

Reviewed-by: Javier Martinez Canillas 

[...]

> +static void ofdrm_crtc_reset(struct drm_crtc *crtc)
> +{
> + struct ofdrm_crtc_state *ofdrm_crtc_state;
> +
> + if (crtc->state) {
> + ofdrm_crtc_state_destroy(to_ofdrm_crtc_state(crtc->state));
> + crtc->state = NULL; /* must be set to NULL here */
> + }
> +
> + ofdrm_crtc_state = kzalloc(sizeof(*ofdrm_crtc_state), GFP_KERNEL);
> + if (!ofdrm_crtc_state)
> + return;
> + __drm_atomic_helper_crtc_reset(crtc, _crtc_state->base);
> +}
> +

IMO this function is hard to read, I would instead write it as following:

static void ofdrm_crtc_reset(struct drm_crtc *crtc)
{
struct ofdrm_crtc_state *ofdrm_crtc_state = 
kzalloc(sizeof(*ofdrm_crtc_state), GFP_KERNEL);

if (!ofdrm_crtc_state)
return;

if (crtc->state) {
ofdrm_crtc_state_destroy(to_ofdrm_crtc_state(crtc->state));
crtc->state = NULL; /* must be set to NULL here */
}

__drm_atomic_helper_crtc_reset(crtc, _crtc_state->base);
}

Also with that form I think that the crtc->state = NULL could just be dropped ?

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 07/10] drm/ofdrm: Add ofdrm for Open Firmware framebuffers

[Javier Martinez Canillas]

Hello Thomas,

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Open Firmware provides basic display output via the 'display' node.
> DT platform code already provides a device that represents the node's
> framebuffer. Add a DRM driver for the device. The display mode and
> color format is pre-initialized by the system's firmware. Runtime
> modesetting via DRM is not possible. The display is useful during
> early boot stages or as error fallback.
> 

I'm not familiar with OF display but the driver looks good to me.

Reviewed-by: Javier Martinez Canillas 

I just have a few questions below.

[...]

> +static int ofdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
> +struct drm_atomic_state 
> *new_state)
> +{
> + struct drm_plane_state *new_plane_state = 
> drm_atomic_get_new_plane_state(new_state, plane);
> + struct drm_crtc_state *new_crtc_state;
> + int ret;
> +
> + if (!new_plane_state->fb)
> + return 0;
> +
> + new_crtc_state = drm_atomic_get_new_crtc_state(new_state, 
> new_plane_state->crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state, 
> new_crtc_state,
> +   DRM_PLANE_HELPER_NO_SCALING,
> +   DRM_PLANE_HELPER_NO_SCALING,
> +   false, false);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}

This seems to be exactly the same check than used in the simpledrm driver.
Maybe could be moved to the fwfb helper library too ?

[...]

> +
> +static void ofdrm_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> +  struct drm_atomic_state *old_state)
> +{
> + /*
> +  * Always enabled; disabling clears the screen in the
> +  * primary plane's atomic_disable function.
> +  */
> +}
> +

Same comment than for simpledrm, are these no-op helpers really needed ?

[...]

> +static const struct of_device_id ofdrm_of_match_display[] = {
> + { .compatible = "display", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ofdrm_of_match_display);
> +

I don't see a binding for this in Documentation/devicetree/bindings/display.
Do we need one or it's that only required for FDT and not Open Firmware DT ?

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 06/10] drm/simpledrm: Move some functionality into fwfb helper library

[Javier Martinez Canillas]

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Move some of simpledrm's functionality into a helper library. Other
> drivers for firmware-provided framebuffers will also need functions
> to handle fixed modes and color formats, or update the back buffer.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Nice patch!

[...]

> +
> +/**
> + * DOC: overview
> + *
> + * The Firmware Framebuffer library FWFB provides helpers for devices with
> + * fixed-mode backing storage. It helps drivers to export a display mode of
> + * te correct size and copy updates to the backing storage.

the

it is "backing storage" or "backing store" ? I always thought that storage was
used for non-volatile media while "store" could be volatile and non-volatile.

[...]

> +/**
> + * drm_fwfb_init - Initializes an fwfb buffer
> + * @fwfb: fwfb buffer
> + * @screen_base: Address of the backing buffer in kernel address space
> + * @width: Number of pixels per scanline
> + * @height: Number of scanlines
> + * @format: Color format
> + * @pitch: Distance between two consecutive scanlines in bytes
> + *
> + * Returns:
> + * 0 on success, or a negative errno code otherwise.
> + */
> +int drm_fwfb_init(struct drm_fwfb *fwfb, struct iosys_map *screen_base,
> +   unsigned int width, unsigned int height,
> +   const struct drm_format_info *format, unsigned int pitch)
> +{
> + fwfb->screen_base = *screen_base;
> + fwfb->mode = drm_fwfb_mode(width, height);
> + fwfb->format = format;

It seems a little bit arbitrary to me that format is the only field that's
a pointer and the other ones are embedded into the struct drm_fwfb. Any
reason for that or is just a consequence of how types were used by the
simpledrm_device_create() function before that code moved into helpers ?

[...]

> +static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, 
> uint32_t fourcc)
> +{
> + const uint32_t *fourccs_end = fourccs + nfourccs;
> +
> + while (fourccs < fourccs_end) {
> + if (*fourccs == fourcc)
> + return true;
> + ++fourccs;
> + }
> + return false;
> +}

This seems a helper that could be useful besides the drm_fwfb_helper.c file.

I believe patches 1-6 shouldn't wait for the others in this series and could
just be merged when ready. Patches 7-10 can follow later.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 05/10] drm/simpledrm: Convert to atomic helpers

[Javier Martinez Canillas]

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Replace the simple-KMS helpers with the regular atomic helpers. The
> regular helpers are better architectured and therefore allow for easier
> code sharing among drivers. No functional changes.
>

Acked-by: Javier Martinez Canillas 

But I've a question below...
 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/tiny/simpledrm.c | 283 ---
>  1 file changed, 180 insertions(+), 103 deletions(-)

[...]

> +static void simpledrm_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> + struct drm_atomic_state 
> *old_state)
> +{
> + /*
> +  * Always enabled; screen updates are performed by
> +  * the primary plane's atomic_update function.
> +  */
> +}
> +
> +static void simpledrm_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> +  struct drm_atomic_state 
> *old_state)
> +{
> + /*
> +  * Always enabled; disabling clears the screen in the
> +  * primary plane's atomic_disable function.
> +  */
> +}

...do we really need to have these ? Can't we just not set them ?

> +
> +static const struct drm_crtc_helper_funcs simpledrm_crtc_helper_funcs = {
> + .mode_valid = simpledrm_crtc_helper_mode_valid,
> + .atomic_check = simpledrm_crtc_helper_atomic_check,
> + .atomic_enable = simpledrm_crtc_helper_atomic_enable,
> + .atomic_disable = simpledrm_crtc_helper_atomic_disable,
> +};
> +
looking at 
https://elixir.bootlin.com/linux/latest/source/include/drm/drm_modeset_helper_vtables.h#L703
that says the .atomic_{en,dis}able handlers are optional.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 04/10] drm/simpledrm: Compute framebuffer stride if not set

[Javier Martinez Canillas]

Hello Geert,

On 7/21/22 16:46, Geert Uytterhoeven wrote:
> Hi Thomas,
> 
> On Wed, Jul 20, 2022 at 4:27 PM Thomas Zimmermann  wrote:
>> Compute the framebuffer's scanline stride length if not given by
>> the simplefb data.
>>
>> Signed-off-by: Thomas Zimmermann 
> 
> Thanks for your patch!
> 
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -743,6 +743,9 @@ static struct simpledrm_device 
>> *simpledrm_device_create(struct drm_driver *drv,
>> drm_err(dev, "no simplefb configuration found\n");
>> return ERR_PTR(-ENODEV);
>> }
>> +   if (!stride)
>> +   stride = format->cpp[0] * width;
> 
> DIV_ROUND_UP(drm_format_info_bpp(format) * width, 8)
>

I think you meant here:

DIV_ROUND_UP(drm_format_info_bpp(format, 0) * width, 8) ?
 
With that change,

Acked-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 03/10] drm/simpledrm: Remove pdev field from device structure

[Javier Martinez Canillas]

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Replace the remaining uses of the field pdev by upcasts from the Linux
> device and remove the field.
> 
> Signed-off-by: Thomas Zimmermann 

Much better indeed.

Acked-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 02/10] drm/simpledrm: Inline device-init helpers

[Javier Martinez Canillas]

Hello Thomas,

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Inline the helpers for initializing the hardware FB, the memory
> management and the modesetting into the device-creation function.
> No functional changes.
>

Could you please elaborate in the commit message why this change is
desirable?  Without this additional context, this feels like going
backwards, since you are dropping few helpers that have quite self
contained code and making simpledrm_device_create() much larger.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 01/10] drm/simpledrm: Remove mem field from device structure

[Javier Martinez Canillas]

On 7/20/22 16:27, Thomas Zimmermann wrote:
> Remove the unused mem field from struct simpledrm_device.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Acked-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 6/6] i2c: Make remove callback return void

[Javier Martinez Canillas]

On 6/29/22 09:55, Christophe Leroy wrote:
> 
> 
> Le 29/06/2022 à 09:23, Uwe Kleine-König a écrit :
>> Hello,
>>
>> [I dropped nearly all individuals from the Cc: list because various
>> bounces reported to be unhappy about the long (logical) line.]
> 
> Good idea, even patchwork made a mess of it, see 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220628140313.74984-7-u.kleine-koe...@pengutronix.de/
> 

FYI, for patches like these what I usually use is:

./scripts/get_maintainer.pl --nogit-fallback --no-m --no-r

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 6/6] i2c: Make remove callback return void

[Javier Martinez Canillas]

On 6/29/22 09:23, Uwe Kleine-König wrote:
> Hello,
> 
> [I dropped nearly all individuals from the Cc: list because various
> bounces reported to be unhappy about the long (logical) line.]
>

Yes, it also bounced for me when I tried to reply earlier today.

> diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c 
> b/drivers/gpu/drm/solomon/ssd130x-i2c.c
> index 1e0fcec7be47..ddfa0bb5d9c9 100644
> --- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
> +++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
> @@ -39,13 +39,11 @@ static int ssd130x_i2c_probe(struct i2c_client *client)
>   return 0;
>  }
>  
> -static int ssd130x_i2c_remove(struct i2c_client *client)
> +static void ssd130x_i2c_remove(struct i2c_client *client)
>  {
>   struct ssd130x_device *ssd130x = i2c_get_clientdata(client);
>  
>   ssd130x_remove(ssd130x);
> -
> - return 0;
>  }
>  
>  static void ssd130x_i2c_shutdown(struct i2c_client *client)

Reviewed-by: Javier Martinez Canillas  
-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers

[Javier Martinez Canillas]

e.clock = mode.hdisplay * mode.vdisplay * 60 / 1000 /* kHz */;

Maybe a comment here about the clock value chosen ?

> + drm_mode_set_name();
> +
> + return mode;
> +}
> +

[snip]

> +
> + /*
> +  * Never use pcim_ or other managed helpers on the returned PCI
> +  * device. Otherwise, probing the native driver will fail for
> +  * resource conflicts. PCI-device management has to be tied to
> +  * the lifetime of the platform device until the native driver
> +  * takes over.
> +  */

Ah, was this the issue that you mentioned the other day? How interesting.


> +/*
> + * Support all formats of OF display and maybe more; in order
> + * of preference. The display's update function will do any
> + * conversion necessary.
> + *
> + * TODO: Add blit helpers for remaining formats and uncomment
> + *   constants.
> + */
> +static const uint32_t ofdrm_default_formats[] = {
> + DRM_FORMAT_XRGB,
> + DRM_FORMAT_RGB565,
> + //DRM_FORMAT_XRGB1555,

Wonder if makes sense to keep this commented and the TODO, why not
leave that format from first version and just do it as follow-up ?

> +static const struct drm_connector_funcs ofdrm_connector_funcs = {
> + .reset = drm_atomic_helper_connector_reset,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};

All of the callbacks used comes from helper libraries so I maybe we
could have a macro or something to set those ? It's the same set that
are used in simpledrm so it would make sense to have them defined in
the same place.

> +static const struct drm_mode_config_funcs ofdrm_mode_config_funcs = {
> + .fb_create = drm_gem_fb_create_with_dirty,
> + .atomic_check = drm_atomic_helper_check,
> + .atomic_commit = drm_atomic_helper_commit,
> +};
> +

Same for these. We could also have a macro to define this for both
simpledrm and ofdrm.

> +static const uint32_t *ofdrm_device_formats(struct ofdrm_device *odev, 
> size_t *nformats_out)
> +{
> + struct drm_device *dev = >dev;
> + size_t i;
> +
> + if (odev->nformats)
> + goto out; /* don't rebuild list on recurring calls */
> +

Nice optimization to cache this.

> + /*
> +  * TODO: The ofdrm driver converts framebuffers to the native
> +  * format when copying them to device memory. If there are more
> +  * formats listed than supported by the driver, the native format
> +  * is not supported by the conversion helpers. Therefore *only*
> +  * support the native format and add a conversion helper ASAP.
> +  */
> + if (drm_WARN_ONCE(dev, i != odev->nformats,
> +   "format conversion helpers required for %p4cc",
> +   >format->format)) {
> + odev->nformats = 1;
> + }
> +

Interesting. Did you find some formats that were not supported ?

The rest of the patch looks good to me, thanks a lot for writing this.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 1/2] MAINTAINERS: Broaden scope of simpledrm entry

[Javier Martinez Canillas]

Hello Thomas,

On 5/18/22 20:30, Thomas Zimmermann wrote:
> There will be more DRM drivers for firmware-provided framebuffers. Use
> the existing entry for simpledrm instead of adding a new one for each
> driver. Also add DRM's aperture helpers, which are part of the driver's
> infrastructure.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

I think you could push this without waiting for 2/2 to be ready.

>  MAINTAINERS | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5c1fd93d9050..43d833273ae9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6388,13 +6388,15 @@ S:Orphan / Obsolete
>  F:   drivers/gpu/drm/savage/
>  F:   include/uapi/drm/savage_drm.h
>  
> -DRM DRIVER FOR SIMPLE FRAMEBUFFERS
> +DRM DRIVER FOR FIRMWARE FRAMEBUFFERS
>  M:   Thomas Zimmermann 
>  M:   Javier Martinez Canillas 
>  L:   dri-de...@lists.freedesktop.org
>  S:   Maintained
>  T:   git git://anongit.freedesktop.org/drm/drm-misc
> +F:   drivers/gpu/drm/drm_aperture.c
>  F:   drivers/gpu/drm/tiny/simpledrm.c
> +F:   include/drm/drm_aperture.h

I wonder if we could add drivers/firmware/sysfb* as well, it certainly is
related since is the place where different platforms register the device.

But it's not in drivers/gpu, hence the question if we could include it 
(and possibly merge it through drm-misc as well, etc).

Dave, Daniel, what do you think ?

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 2/2] fbdev: Warn in hot-unplug workaround for framebuffers without device

[Javier Martinez Canillas]

Hello Thomas,

Thanks a lot for re-spinning your series.

On 4/19/22 12:04, Thomas Zimmermann wrote:
> A workaround makes fbdev hot-unplugging work for framebuffers without
> device. The only user for this feature was offb. As each OF framebuffer
> now has an associated platform device, the workaround hould no longer
> be triggered. Update it with a warning and rewrite the comment. Fbdev
> drivers that trigger the hot-unplug workaround really need to be fixed.
> 
> Signed-off-by: Thomas Zimmermann 
> Suggested-by: Javier Martinez Canillas 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 2/2] fbdev: Remove hot-unplug workaround for framebuffers without device

[Javier Martinez Canillas]

Hello Thomas,

On 4/13/22 20:09, Thomas Zimmermann wrote:

[snip]

>>>> index bc6ed750e915..bdd00d381bbc 100644
>>>> --- a/drivers/video/fbdev/core/fbmem.c
>>>> +++ b/drivers/video/fbdev/core/fbmem.c
>>>> @@ -1579,14 +1579,7 @@ static void 
>>>> do_remove_conflicting_framebuffers(struct apertures_struct *a,
>>>> * If it's not a platform device, at least print a 
>>>> warning. A
>>>> * fix would add code to remove the device from the 
>>>> system.
>>>> */
>>>> -  if (!device) {
>>>> -  /* TODO: Represent each OF framebuffer as its 
>>>> own
>>>> -   * device in the device hierarchy. For now, offb
>>>> -   * doesn't have such a device, so unregister the
>>>> -   * framebuffer as before without warning.
>>>> -   */
>>>> -  do_unregister_framebuffer(registered_fb[i]);
>>>
>>> Maybe we could still keep this for a couple of releases but with a big
>>> warning that's not supported in case there are out-of-tree drivers out
>>> there that still do this ?
>>>
>>> Or at least a warning if the do_unregister_framebuffer() call is removed.
>>
>> Yeah dying while holding console_lock isn't fun, and not having a WARN_ON
>> + bail-out code pretty much forces bug reporters to do a bisect here to
>> give us something more than "machine dies at boot with no messages".
>>
>> I'd just outright keep the WARN_ON here for 1-2 years even to really make
>> sure we got all the bug reports, since often these older machines only
>> update onto LTS releases.
> 
> If that's what the consent is, I'll go with it.
> 
> I'm just not sure if we talk about the same problem. offb didn't have a 
> platform device, so we recently added this workaround with 'if 
> (!device)'.  All the other fbdev drivers have a platform device; and 
> anything else that could fail is out-of-tree. We don't really care about 
> those AFAIK.
>

Yes, agreed on the offb change but I'm not really sure if we don't care
about out-of-tree modules. I mean, you are right in theory but I still
feel that we are changing a core behavior without giving people time to
sort out if needed.

Since before commit 27599aacbaef ("fbdev: Hot-unplug firmware fb devices
on forced removal") registered FBs didn't need to have a device, but now
that will lead to a NULL pointer dereference in dev_is_platform(device).

And that change only landed in v5.18-rc1, so it is fairly recent.

I know that we follow 
https://www.kernel.org/doc/Documentation/process/stable-api-nonsense.rst
but still my opinion is that having a warning for a couple of releases
if registered_fb[i]->device is NULL, instead of just crashing would be
a better way to handle this.
 
> With offb converted, we could practically remove all of the checks here 
> and call platform_device_unregister() unconditionally.
>

Yes for mainline, but as mentioned I thought mostly about out-of-tree. If
folks agree that we shouldn't care about these, I'm Ok with that as well.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 1/2] of: Create platform devices for OF framebuffers

[Javier Martinez Canillas]

On 4/13/22 19:58, Thomas Zimmermann wrote:
> Hi

[snip]

>>>
>>>  /* Populate everything else. */
>>>  of_platform_default_populate(NULL, NULL, NULL);
>>
>> I'm pretty sure it's just this call that's the problem for PPC though
>> none of the above existed when adding this caused a regression. Can we
>> remove the ifdef and just make this call conditional on
>> !IS_ENABLED(CONFIG_PPC).
> 
> Together with the changes in of_platform_populate_framebuffers(), the 
> code is more or less an "if-else" depending on PPC. I'll drop 
> of_platform_populate_framebuffers() from the patch and make a separate 
> implementation of of_platform_default_populate_init for PPC. Seems like 
> the easiest solution to me.
>

That sounds reasonable to me as well. Feel free to retain my R-B tag
when posting v2.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 2/2] fbdev: Remove hot-unplug workaround for framebuffers without device

[Javier Martinez Canillas]

On 4/13/22 11:24, Thomas Zimmermann wrote:
> A workaround makes fbdev hot-unplugging work for framebuffers without
> device. The only user for this feature was offb. As each OF framebuffer
> now has an associated platform device, the workaround is no longer
> needed. Remove it. Effectively reverts commit 0f525289ff0d ("fbdev: Fix
> unregistering of framebuffers without device").
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/video/fbdev/core/fbmem.c | 9 +
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fbmem.c 
> b/drivers/video/fbdev/core/fbmem.c
> index bc6ed750e915..bdd00d381bbc 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1579,14 +1579,7 @@ static void do_remove_conflicting_framebuffers(struct 
> apertures_struct *a,
>* If it's not a platform device, at least print a 
> warning. A
>* fix would add code to remove the device from the 
> system.
>*/
> - if (!device) {
> - /* TODO: Represent each OF framebuffer as its 
> own
> -  * device in the device hierarchy. For now, offb
> -  * doesn't have such a device, so unregister the
> -  * framebuffer as before without warning.
> -  */
> - do_unregister_framebuffer(registered_fb[i]);

Maybe we could still keep this for a couple of releases but with a big
warning that's not supported in case there are out-of-tree drivers out
there that still do this ?

Or at least a warning if the do_unregister_framebuffer() call is removed.

Regardless of what you chose to do, the patch looks good to me.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH 1/2] of: Create platform devices for OF framebuffers

[Javier Martinez Canillas]

Hello Thomas,

Thanks for working on this.

On 4/13/22 11:24, Thomas Zimmermann wrote:
> Create a platform device for each OF-declared framebuffer and have
> offb bind to these devices. Allows for real hot-unplugging and other
> drivers besides offb.
> 
> Originally, offb created framebuffer devices while initializing its
> module by parsing the OF device tree. No actual Linux device was set
> up. This tied OF framebuffers to offb and makes writing other drivers
> for the OF framebuffers complicated. The absence of a Linux device
> prevented real hot-unplugging. Adding a distinct platform device for
> each OF framebuffer solves both problems. Specifically, a DRM drivers
> can now provide graphics output with modern userspace.
> 
> Some of the offb init code is now located in the OF initialization.
> There's now also an implementation of of_platform_default_populate_init(),
> which was missing before. The OF side creates different devices for
> either OF display nodes or bootx displays as they require different
> handling by the driver. The offb drivers picks up each type of device
> and runs the appropriate fbdev initialization.
> 
> Tested with OF display nodes on qemu's ppc64le target.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

[snip]

> + for_each_node_by_type(node, "display") {
> + if (!of_get_property(node, "linux,opened", NULL) ||
> + !of_get_property(node, "linux,boot-display", NULL))
> + continue;
> + dev = of_platform_device_create(node, "of-display", NULL);
> + if (WARN_ON(!dev))
> + return -ENOMEM;
> + boot_display = node;
> + break;
> + }
> + for_each_node_by_type(node, "display") {
> + if (!of_get_property(node, "linux,opened", NULL) || node == 
> boot_display)
> + continue;
> +     of_platform_device_create(node, "of-display", NULL);

Shouldn't check for the return value here too ?

Other than this small nit, it looks good to me.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Geert,

On Wed, Aug 30, 2017 at 10:15 PM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> Hi Javier,
>
> On Wed, Aug 30, 2017 at 9:57 PM, Javier Martinez Canillas
> <jav...@dowhile0.org> wrote:
>>> I think we should talk about the same case: Let me repeat what I did:
>>>
>>> 1) I added your patch "eeprom: at24: Add OF device ID table"
>>> 2) I added an EEPROM node to an I2C
>>>
>>> +   eeprom@50 {
>>> +   compatible = "renesas,24c01";
>>> +   reg = <0x50>;
>>> +   };
>>>
>>> -> no at24 binding to the device
>>>
>>> 3) I revert your patch
>>>
>>> -> at24 binding to the device
>>>
>>
>> I've tested this and you are right, it fails...
>>
>> The problem is that the patch also changes how the driver obtains the
>> EEPROM parameters (the magic value in the entry's data field).
>>
>> So even when module autoload and device / driver matching works, the
>> driver probe function fails because if (client->dev.of_node) the
>> driver attempts to get the entry data using
>> of_device_get_match_data(), which is obviously wrong since the
>> compatible string in the dev node isn't present in the OF table.
>>
>> The id->driver_data from the I2C table should be used instead since
>> that's the table that matches in this case.
>>
>> One option is to fallback to id->driver_data if
>> of_device_get_match_data() fails, but that's just an (ugly)
>> workaround. So I agree with you that the best option is to wait for
>> the DTS patches to land first.
>
> Which means new kernels won't work with old DTBs. Oops...
> I'm afraid that needs to be fixed.  People care about DTB backward
> compatibility on many platforms.
>

Right, I've yet to find one of those mythical platforms that ship old
DTBs with new kernels, but I agree with you since people seem to care
about backward compatibility (at least on theory).

So I see two options then:

1) Use the workaround I mentioned and lookup the I2C device ID table
entry data if of_device_get_match_data() fails

2) Only call of_device_get_match_data() if (dev->of_node &&
of_match_device(dev->driver->of_match_table, dev))

Not sure what's the preferred idiom for these cases.

To good thing about keeping backward compatibility is that Wolfram
would be able to pick the driver patch even before the DTS patches
land.

Best regards,
Javier

Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

>
> I think we should talk about the same case: Let me repeat what I did:
>
> 1) I added your patch "eeprom: at24: Add OF device ID table"
> 2) I added an EEPROM node to an I2C
>
> +   eeprom@50 {
> +   compatible = "renesas,24c01";
> +   reg = <0x50>;
> +   };
>
> -> no at24 binding to the device
>
> 3) I revert your patch
>
> -> at24 binding to the device
>

I've tested this and you are right, it fails...

The problem is that the patch also changes how the driver obtains the
EEPROM parameters (the magic value in the entry's data field).

So even when module autoload and device / driver matching works, the
driver probe function fails because if (client->dev.of_node) the
driver attempts to get the entry data using
of_device_get_match_data(), which is obviously wrong since the
compatible string in the dev node isn't present in the OF table.

The id->driver_data from the I2C table should be used instead since
that's the table that matches in this case.

One option is to fallback to id->driver_data if
of_device_get_match_data() fails, but that's just an (ugly)
workaround. So I agree with you that the best option is to wait for
the DTS patches to land first.

It worked for me on my previous tests because the tested drivers
didn't use a table entry data, I'm so sorry for missing this :(

Best regards,
Javier

Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

On Tue, Aug 29, 2017 at 10:48 AM, Wolfram Sang  wrote:
>
>> I don't have a DT based system at hand now, but I'll test it again and
>> let you know probably tomorrow.
>
> I will try again today, too. Thanks!
>

Ok, I had some time to do some tests again. I used an ARM Chromebook
(Exynos Peach Pi) that has an I2C touchpad (Atmel maXTouch).

Tested the following cases:

1) Driver without OF device ID table (only a I2C table with a
"maxtouch" entry) and DTS defining a device node with a
"atmel,maxtouch" compatible string. This is the case without any of
the patches in this series.

$ modinfo drivers/input/touchscreen/atmel_mxt_ts.ko | grep maxtouch
alias:  i2c:maxtouch

$ grep maxtouch /sys/devices/platform/soc/12e0.i2c/i2c-8/8-004b/uevent
OF_COMPATIBLE_0=atmel,maxtouch
MODALIAS=i2c:maxtouch

2) Driver without OF device ID table (only a I2C table with a
"maxtouch" entry) and DTS defining a device node with a
"atmel,maxtouch", "generic,maxtouch" compatible string. This is the
case when platform maintainers merge the DTS patches without the
driver patch.

$ modinfo drivers/input/touchscreen/atmel_mxt_ts.ko | grep maxtouch
alias:  i2c:maxtouch

$ grep maxtouch /sys/devices/platform/soc/12e0.i2c/i2c-8/8-004b/uevent
OF_COMPATIBLE_0=atmel,maxtouch
OF_COMPATIBLE_1=generic,maxtouch
MODALIAS=i2c:maxtouch

3) Driver with an OF device ID table (with a "generic,maxtouch" entry)
and DTS defining a device node with a "atmel,maxtouch" compatible
string. This is the case when the driver patch is merged without the
DTS patches.

$ modinfo drivers/input/touchscreen/atmel_mxt_ts.ko | grep maxtouch
alias:  of:N*T*Cgeneric,maxtouchC*
alias:  of:N*T*Cgeneric,maxtouch
alias:  i2c:maxtouch

$ grep maxtouch /sys/devices/platform/soc/12e0.i2c/i2c-8/8-004b/uevent
OF_COMPATIBLE_0=atmel,maxtouch
MODALIAS=i2c:maxtouch

4) Driver with an OF device ID table (with a "generic,maxtouch" entry)
and DTS defining a device node with a "atmel,maxtouch",
"generic,maxtouch" compatible string. This is the case when both the
DTS and driver patches are merged.

$ modinfo drivers/input/touchscreen/atmel_mxt_ts.ko | grep maxtouch
alias:  of:N*T*Cgeneric,maxtouchC*
alias:  of:N*T*Cgeneric,maxtouch
alias:  i2c:maxtouch

$ grep maxtouch /sys/devices/platform/soc/12e0.i2c/i2c-8/8-004b/uevent
OF_COMPATIBLE_0=atmel,maxtouch
OF_COMPATIBLE_1=generic,maxtouch
MODALIAS=i2c:maxtouch

For all cases module autoload, driver probe and evtest worked for me.

You said that (3) doesn't work but I don't understand why is failing
for you. Probably I'm missing something.

Best regards,
Javier

Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

On Mon, Aug 28, 2017 at 6:01 PM, Wolfram Sang  wrote:
>
>> > But there is a dependency, no? If I apply the driver patch,
>> > non-converted device trees will not find their eeproms anymore. So, I
>>
>> I don't think that's correct. If you apply this patch before the DTS
>> changes, the driver will still match using the I2C device ID table
>> like it has been doing it until today.
>
> My tests do not confirm this. If I add a node with a "renesas,24c01"
> compatible to my board, it works before your patch, but not after. If I
> change it to "atmel,24c01" it works even after your patch. I haven't
> looked into it, though, maybe i2c_of_match_device_sysfs() is stepping on
> our foots here?
>
> Did you test and did it work for you?
>

I would swear that I tested both combinations (driver patch without DT
changes and DTS changes without driver patch), but it was months ago
when I first posted the patches so I may misremembering.

I don't have a DT based system at hand now, but I'll test it again and
let you know probably tomorrow.

Best regards,
Javier

Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

On Mon, Jul 31, 2017 at 5:30 PM, Wolfram Sang  wrote:
>
>> Patches can be applied independently since the DTS changes without driver
>> changes are no-op and the OF table won't be used without the DTS changes.
>
> But there is a dependency, no? If I apply the driver patch,
> non-converted device trees will not find their eeproms anymore. So, I

I don't think that's correct. If you apply this patch before the DTS
changes, the driver will still match using the I2C device ID table
like it has been doing it until today.

IOW, this is what will happen:

1- an OF device is registered with the wrong compatible (not found in
the OF table)
2- the I2C core strips the vendor part and fills the struct i2c_client
.name with the device part.
3- i2c_device_match() will be called since a new device has been registered
4- i2c_of_match_device() will fail because there's no OF entry that
matches the device compatible
5- the I2C core fallbacks to i2c_match_id() and matches using the I2C
device ID table.

So no noticeable difference AFAICT in that case.

Best regards,
Javier

Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

On Thu, Jun 15, 2017 at 8:54 PM, Javier Martinez Canillas
<jav...@dowhile0.org> wrote:
>
> This series is a follow-up to patch [0] that added an OF device ID table
> to the at24 EEPROM driver. As you suggested [1], this version instead of
> adding entries for every used <vendor,device> tuple, only adds a single
> entry for each chip type using the "atmel" vendor as a generic fallback.
>
> The first patch documents in the DT binding what's the correct vendor to
> use and what are the ones that are being deprecated. The second one adds
> the OF device ID table for the at24 driver and the next patches use this
> vendor in the compatible string to each DTS that defines a compatible I2C
> EEPROM device node.
>
> Patches can be applied independently since the DTS changes without driver
> changes are no-op and the OF table won't be used without the DTS changes.
>
> [0]: https://lkml.org/lkml/2017/3/14/589
> [1]: https://lkml.org/lkml/2017/3/15/99
>

Are you planning to pick this series? It has been in the list for
months and were resent many times...

Best regards,
Javier

[RESEND PATCH v5 16/16] powerpc/44x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5: None
Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/warp.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/warp.dts b/arch/powerpc/boot/dts/warp.dts
index e576ee85c42f..ea9053ef4819 100644
--- a/arch/powerpc/boot/dts/warp.dts
+++ b/arch/powerpc/boot/dts/warp.dts
@@ -238,7 +238,7 @@
 
/* This will create 52 and 53 */
at24@52 {
-   compatible = "at,24c04";
+   compatible = "atmel,24c04";
reg = <0x52>;
};
};
-- 
2.9.3

[RESEND PATCH v5 15/16] powerpc/83xx: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5:
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc8308_p1m.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8349emitx.dts | 4 ++--
 arch/powerpc/boot/dts/mpc8377_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8377_wlan.dts | 2 +-
 arch/powerpc/boot/dts/mpc8378_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8379_rdb.dts  | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts 
b/arch/powerpc/boot/dts/mpc8308_p1m.dts
index 57f86cdf9f36..cab933b3957a 100644
--- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
+++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
@@ -123,7 +123,7 @@
interrupt-parent = <>;
dfsrr;
fram@50 {
-   compatible = "ramtron,24c64";
+   compatible = "ramtron,24c64", "atmel,24c64";
reg = <0x50>;
};
};
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 90aed3ac2f69..648a85858eb5 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -92,7 +92,7 @@
dfsrr;
 
eeprom: at24@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256", "atmel,24c256";
reg = <0x50>;
};
 
@@ -130,7 +130,7 @@
};
 
spd: at24@51 {
-   compatible = "at24,spd";
+   compatible = "atmel,spd";
reg = <0x51>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_rdb.dts 
b/arch/powerpc/boot/dts/mpc8377_rdb.dts
index e32613963ab0..5e85d8c93bca 100644
--- a/arch/powerpc/boot/dts/mpc8377_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8377_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_wlan.dts 
b/arch/powerpc/boot/dts/mpc8377_wlan.dts
index c0c790168b96..fee15fcbb46f 100644
--- a/arch/powerpc/boot/dts/mpc8377_wlan.dts
+++ b/arch/powerpc/boot/dts/mpc8377_wlan.dts
@@ -135,7 +135,7 @@
dfsrr;
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8378_rdb.dts 
b/arch/powerpc/boot/dts/mpc8378_rdb.dts
index 71842fcd621f..e973d61956b9 100644
--- a/arch/powerpc/boot/dts/mpc8378_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8378_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8379_rdb.dts 
b/arch/powerpc/boot/dts/mpc8379_rdb.dts
index e442a29b2fe0..ed5d12ff2ee0 100644
--- a/arch/powerpc/boot/dts/mpc8379_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8379_rdb.dts
@@ -148,7 +148,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
-- 
2.9.3

[RESEND PATCH v5 14/16] powerpc/512x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5: None
Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc5121ads.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts 
b/arch/powerpc/boot/dts/mpc5121ads.dts
index 75888ce2c792..fcaa9bad4bda 100644
--- a/arch/powerpc/boot/dts/mpc5121ads.dts
+++ b/arch/powerpc/boot/dts/mpc5121ads.dts
@@ -94,7 +94,7 @@
};
 
eeprom@50 {
-   compatible = "at,24c32";
+   compatible = "atmel,24c32";
reg = <0x50>;
};
 
-- 
2.9.3

[RESEND PATCH v5 13/16] powerpc/fsl: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5:
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/fsl/b4qds.dtsi|  8 
 arch/powerpc/boot/dts/fsl/c293pcie.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p1010rdb.dtsi |  2 +-
 arch/powerpc/boot/dts/fsl/p1023rdb.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p2041rdb.dts  |  4 ++--
 arch/powerpc/boot/dts/fsl/p3041ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p4080ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5020ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5040ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/t208xqds.dtsi |  8 
 arch/powerpc/boot/dts/fsl/t4240qds.dts  | 12 ++--
 arch/powerpc/boot/dts/fsl/t4240rdb.dts  |  6 +++---
 12 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/b4qds.dtsi 
b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
index 3785ef826d07..999efd3bc167 100644
--- a/arch/powerpc/boot/dts/fsl/b4qds.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
@@ -166,19 +166,19 @@
reg = <0>;
 
eeprom@50 {
-   compatible = "at24,24c64";
+   compatible = "atmel,24c64";
reg = <0x50>;
};
eeprom@51 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x51>;
};
eeprom@53 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x53>;
};
eeprom@57 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x57>;
};
rtc@68 {
diff --git a/arch/powerpc/boot/dts/fsl/c293pcie.dts 
b/arch/powerpc/boot/dts/fsl/c293pcie.dts
index 66709788429d..5e905e0857cf 100644
--- a/arch/powerpc/boot/dts/fsl/c293pcie.dts
+++ b/arch/powerpc/boot/dts/fsl/c293pcie.dts
@@ -153,7 +153,7 @@
  {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c1024";
+   compatible = "st,24c1024", "atmel,24c1024";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi 
b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
index a8e4ba070104..2ca9cee2ddeb 100644
--- a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
@@ -89,7 +89,7 @@
 _soc {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256", "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1023rdb.dts 
b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
index 9716ca64651c..ead928364beb 100644
--- a/arch/powerpc/boot/dts/fsl/p1023rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
@@ -79,7 +79,7 @@
 
i2c@3000 {
eeprom@53 {
-   compatible = "at24,24c04";
+   compatible = "atmel,24c04";
reg = <0x53>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p2041rdb.dts 
b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
index e50fea95a853..950816b9d6e1

[RESEND PATCH v5 12/16] powerpc/5200: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5:
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/digsy_mtc.dts | 2 +-
 arch/powerpc/boot/dts/pcm030.dts| 2 +-
 arch/powerpc/boot/dts/pcm032.dts| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/boot/dts/digsy_mtc.dts 
b/arch/powerpc/boot/dts/digsy_mtc.dts
index 955bff629df3..c280e75c86bf 100644
--- a/arch/powerpc/boot/dts/digsy_mtc.dts
+++ b/arch/powerpc/boot/dts/digsy_mtc.dts
@@ -73,7 +73,7 @@
 
i2c@3d00 {
eeprom@50 {
-   compatible = "at,24c08";
+   compatible = "atmel,24c08";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 192e66af0001..836e47cc4bed 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -71,7 +71,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32", "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm032.dts
index 96b139bf50e9..576249bf2fb9 100644
--- a/arch/powerpc/boot/dts/pcm032.dts
+++ b/arch/powerpc/boot/dts/pcm032.dts
@@ -75,7 +75,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32", "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
-- 
2.9.3

[RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

This series is a follow-up to patch [0] that added an OF device ID table
to the at24 EEPROM driver. As you suggested [1], this version instead of
adding entries for every used <vendor,device> tuple, only adds a single
entry for each chip type using the "atmel" vendor as a generic fallback.

The first patch documents in the DT binding what's the correct vendor to
use and what are the ones that are being deprecated. The second one adds
the OF device ID table for the at24 driver and the next patches use this
vendor in the compatible string to each DTS that defines a compatible I2C
EEPROM device node.

Patches can be applied independently since the DTS changes without driver
changes are no-op and the OF table won't be used without the DTS changes.

[0]: https://lkml.org/lkml/2017/3/14/589
[1]: https://lkml.org/lkml/2017/3/15/99

Best regards,
Javier

Changes in v5:
- Only deprecate the atmel variants at25 and at (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Document the manufacturers that have been deprecated (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3:
- Fix wrong .data values for "atmel,24c02" and "atmel,24c64" entries.
- Add Geert Uytterhoeven reviewed-by tag.
- Add Geert Uytterhoeven reviewed-by tag.

Changes in v2:
- Only add a single OF device ID entry for each device type (Wolfram Sang).

Javier Martinez Canillas (16):
  dt-bindings: i2c: eeprom: Document vendor to be used and deprecated
ones
  eeprom: at24: Add OF device ID table
  ARM: dts: efm32: Add generic compatible string for I2C EEPROM
  ARM: dts: keystone: Add generic compatible string for I2C EEPROM
  ARM: dts: lpc18xx: Add generic compatible string for I2C EEPROM
  ARM: dts: r7s72100: Add generic compatible string for I2C EEPROM
  ARM: dts: koelsch: Add generic compatible string for I2C EEPROM
  ARM: dts: socfpga: Add generic compatible string for I2C EEPROM
  ARM: dts: uniphier: Add generic compatible string for I2C EEPROM
  ARM: dts: zynq: Add generic compatible string for I2C EEPROM
  arm64: zynqmp: Add generic compatible string for I2C EEPROM
  powerpc/5200: Add generic compatible string for I2C EEPROM
  powerpc/fsl: Add generic compatible string for I2C EEPROM
  powerpc/512x: Add generic compatible string for I2C EEPROM
  powerpc/83xx: Add generic compatible string for I2C EEPROM
  powerpc/44x: Add generic compatible string for I2C EEPROM

 .../devicetree/bindings/eeprom/eeprom.txt  |  6 +-
 arch/arm/boot/dts/efm32gg-dk3750.dts   |  2 +-
 arch/arm/boot/dts/keystone-k2e-evm.dts |  2 +-
 arch/arm/boot/dts/keystone-k2hk-evm.dts|  2 +-
 arch/arm/boot/dts/keystone-k2l-evm.dts |  2 +-
 arch/arm/boot/dts/lpc4337-ciaa.dts   

Re: [PATCH v5 00/20] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

On Tue, May 23, 2017 at 3:34 PM, Javier Martinez Canillas
<jav...@dowhile0.org> wrote:
>
> This series is a follow-up to patch [0] that added an OF device ID table
> to the at24 EEPROM driver. As you suggested [1], this version instead of
> adding entries for every used <vendor,device> tuple, only adds a single
> entry for each chip type using the "atmel" vendor as a generic fallback.
>
> The first patch documents in the DT binding what's the correct vendor to
> use and what are the ones that are being deprecated. The second one adds
> the OF device ID table for the at24 driver and the next patches use this
> vendor in the compatible string to each DTS that defines a compatible I2C
> EEPROM device node.
>
> Patches can be applied independently since the DTS changes without driver
> changes are no-op and the OF table won't be used without the DTS changes.
>
> [0]: https://lkml.org/lkml/2017/3/14/589
> [1]: https://lkml.org/lkml/2017/3/15/99
>

Any comments on this series?

Best regards,
Javier

Re: [PATCH v4 19/20] powerpc/83xx: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

Hello Rob,

On Tue, May 23, 2017 at 3:42 PM, Rob Herring <r...@kernel.org> wrote:
> On Mon, May 22, 2017 at 9:02 AM, Javier Martinez Canillas
> <jav...@dowhile0.org> wrote:
>> The at24 driver allows to register I2C EEPROM chips using different vendor
>> and devices, but the I2C subsystem does not take the vendor into account
>> when matching using the I2C table since it only has device entries.
>>
>> But when matching using an OF table, both the vendor and device has to be
>> taken into account so the driver defines only a set of compatible strings
>> using the "atmel" vendor as a generic fallback for compatible I2C devices.
>>
>> So add this generic fallback to the device node compatible string to make
>> the device to match the driver using the OF device ID table.
>>
>> Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>
>>
>> ---
>>
>> Changes in v4:
>> - Only use the atmel manufacturer in the compatible string instead of
>>   keeping the deprecated ones (Rob Herring).
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>>  arch/powerpc/boot/dts/mpc8308_p1m.dts  | 2 +-
>>  arch/powerpc/boot/dts/mpc8349emitx.dts | 4 ++--
>>  arch/powerpc/boot/dts/mpc8377_rdb.dts  | 2 +-
>>  arch/powerpc/boot/dts/mpc8377_wlan.dts | 2 +-
>>  arch/powerpc/boot/dts/mpc8378_rdb.dts  | 2 +-
>>  arch/powerpc/boot/dts/mpc8379_rdb.dts  | 2 +-
>>  6 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts 
>> b/arch/powerpc/boot/dts/mpc8308_p1m.dts
>> index 57f86cdf9f36..702ab4fc5b4a 100644
>> --- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
>> +++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
>> @@ -123,7 +123,7 @@
>> interrupt-parent = <>;
>> dfsrr;
>> fram@50 {
>> -   compatible = "ramtron,24c64";
>> +   compatible = "atmel,24c64";
>
> This should be '"ramtron,24c64", "atmel,24c64"'
>

Yes, I (hopefully) fixed all the occurrences in the v5 that I posted
today, you are cc'ed on that series too.

Again, sorry for misunderstanding your comment on v3.

Best regards,
Javier

[PATCH v5 20/20] powerpc/44x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5: None
Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/warp.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/warp.dts b/arch/powerpc/boot/dts/warp.dts
index e576ee85c42f..ea9053ef4819 100644
--- a/arch/powerpc/boot/dts/warp.dts
+++ b/arch/powerpc/boot/dts/warp.dts
@@ -238,7 +238,7 @@
 
/* This will create 52 and 53 */
at24@52 {
-   compatible = "at,24c04";
+   compatible = "atmel,24c04";
reg = <0x52>;
};
};
-- 
2.9.3

[PATCH v5 19/20] powerpc/83xx: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5:
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc8308_p1m.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8349emitx.dts | 4 ++--
 arch/powerpc/boot/dts/mpc8377_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8377_wlan.dts | 2 +-
 arch/powerpc/boot/dts/mpc8378_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8379_rdb.dts  | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts 
b/arch/powerpc/boot/dts/mpc8308_p1m.dts
index 57f86cdf9f36..cab933b3957a 100644
--- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
+++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
@@ -123,7 +123,7 @@
interrupt-parent = <>;
dfsrr;
fram@50 {
-   compatible = "ramtron,24c64";
+   compatible = "ramtron,24c64", "atmel,24c64";
reg = <0x50>;
};
};
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 90aed3ac2f69..648a85858eb5 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -92,7 +92,7 @@
dfsrr;
 
eeprom: at24@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256", "atmel,24c256";
reg = <0x50>;
};
 
@@ -130,7 +130,7 @@
};
 
spd: at24@51 {
-   compatible = "at24,spd";
+   compatible = "atmel,spd";
reg = <0x51>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_rdb.dts 
b/arch/powerpc/boot/dts/mpc8377_rdb.dts
index e32613963ab0..5e85d8c93bca 100644
--- a/arch/powerpc/boot/dts/mpc8377_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8377_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_wlan.dts 
b/arch/powerpc/boot/dts/mpc8377_wlan.dts
index c0c790168b96..fee15fcbb46f 100644
--- a/arch/powerpc/boot/dts/mpc8377_wlan.dts
+++ b/arch/powerpc/boot/dts/mpc8377_wlan.dts
@@ -135,7 +135,7 @@
dfsrr;
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8378_rdb.dts 
b/arch/powerpc/boot/dts/mpc8378_rdb.dts
index 71842fcd621f..e973d61956b9 100644
--- a/arch/powerpc/boot/dts/mpc8378_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8378_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8379_rdb.dts 
b/arch/powerpc/boot/dts/mpc8379_rdb.dts
index e442a29b2fe0..ed5d12ff2ee0 100644
--- a/arch/powerpc/boot/dts/mpc8379_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8379_rdb.dts
@@ -148,7 +148,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v5 18/20] powerpc/512x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5: None
Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc5121ads.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts 
b/arch/powerpc/boot/dts/mpc5121ads.dts
index 75888ce2c792..fcaa9bad4bda 100644
--- a/arch/powerpc/boot/dts/mpc5121ads.dts
+++ b/arch/powerpc/boot/dts/mpc5121ads.dts
@@ -94,7 +94,7 @@
};
 
eeprom@50 {
-   compatible = "at,24c32";
+   compatible = "atmel,24c32";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v5 17/20] powerpc/fsl: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5:
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/fsl/b4qds.dtsi|  8 
 arch/powerpc/boot/dts/fsl/c293pcie.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p1010rdb.dtsi |  2 +-
 arch/powerpc/boot/dts/fsl/p1023rdb.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p2041rdb.dts  |  4 ++--
 arch/powerpc/boot/dts/fsl/p3041ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p4080ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5020ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5040ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/t208xqds.dtsi |  8 
 arch/powerpc/boot/dts/fsl/t4240qds.dts  | 12 ++--
 arch/powerpc/boot/dts/fsl/t4240rdb.dts  |  6 +++---
 12 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/b4qds.dtsi 
b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
index 3785ef826d07..999efd3bc167 100644
--- a/arch/powerpc/boot/dts/fsl/b4qds.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
@@ -166,19 +166,19 @@
reg = <0>;
 
eeprom@50 {
-   compatible = "at24,24c64";
+   compatible = "atmel,24c64";
reg = <0x50>;
};
eeprom@51 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x51>;
};
eeprom@53 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x53>;
};
eeprom@57 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x57>;
};
rtc@68 {
diff --git a/arch/powerpc/boot/dts/fsl/c293pcie.dts 
b/arch/powerpc/boot/dts/fsl/c293pcie.dts
index 66709788429d..5e905e0857cf 100644
--- a/arch/powerpc/boot/dts/fsl/c293pcie.dts
+++ b/arch/powerpc/boot/dts/fsl/c293pcie.dts
@@ -153,7 +153,7 @@
  {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c1024";
+   compatible = "st,24c1024", "atmel,24c1024";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi 
b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
index a8e4ba070104..2ca9cee2ddeb 100644
--- a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
@@ -89,7 +89,7 @@
 _soc {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256", "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1023rdb.dts 
b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
index 9716ca64651c..ead928364beb 100644
--- a/arch/powerpc/boot/dts/fsl/p1023rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
@@ -79,7 +79,7 @@
 
i2c@3000 {
eeprom@53 {
-   compatible = "at24,24c04";
+   compatible = "atmel,24c04";
reg = <0x53>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p2041rdb.dts 
b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
index e50fea95a853..950816b9d6e1

[PATCH v5 16/20] powerpc/5200: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v5:
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/digsy_mtc.dts | 2 +-
 arch/powerpc/boot/dts/pcm030.dts| 2 +-
 arch/powerpc/boot/dts/pcm032.dts| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/boot/dts/digsy_mtc.dts 
b/arch/powerpc/boot/dts/digsy_mtc.dts
index 955bff629df3..c280e75c86bf 100644
--- a/arch/powerpc/boot/dts/digsy_mtc.dts
+++ b/arch/powerpc/boot/dts/digsy_mtc.dts
@@ -73,7 +73,7 @@
 
i2c@3d00 {
eeprom@50 {
-   compatible = "at,24c08";
+   compatible = "atmel,24c08";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 192e66af0001..836e47cc4bed 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -71,7 +71,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32", "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm032.dts
index 96b139bf50e9..576249bf2fb9 100644
--- a/arch/powerpc/boot/dts/pcm032.dts
+++ b/arch/powerpc/boot/dts/pcm032.dts
@@ -75,7 +75,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32", "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
-- 
2.9.3

[PATCH v5 00/20] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

This series is a follow-up to patch [0] that added an OF device ID table
to the at24 EEPROM driver. As you suggested [1], this version instead of
adding entries for every used <vendor,device> tuple, only adds a single
entry for each chip type using the "atmel" vendor as a generic fallback.

The first patch documents in the DT binding what's the correct vendor to
use and what are the ones that are being deprecated. The second one adds
the OF device ID table for the at24 driver and the next patches use this
vendor in the compatible string to each DTS that defines a compatible I2C
EEPROM device node.

Patches can be applied independently since the DTS changes without driver
changes are no-op and the OF table won't be used without the DTS changes.

[0]: https://lkml.org/lkml/2017/3/14/589
[1]: https://lkml.org/lkml/2017/3/15/99

Best regards,
Javier

Changes in v5:
- Only deprecate the atmel variants at25 and at (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).
- Only replace atmel variant but keep other EEPROM vendors (Geert Uytterhoeven).

Changes in v4:
- Document the manufacturers that have been deprecated (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3:
- Fix wrong .data values for "atmel,24c02" and "atmel,24c64" entries.
- Add Geert Uytterhoeven reviewed-by tag.
- Add Geert Uytterhoeven reviewed-by tag.

Changes in v2:
- Only add a single OF device ID entry for each device type (Wolfram Sang).

Javier Martinez Canillas (20):
  dt-bindings: i2c: eeprom: Document vendor to be used and deprecated
ones
  eeprom: at24: Add OF device ID table
  ARM: dts: omap: Add generic compatible string for I2C EEPROM
  ARM: dts: turris-omnia: Add generic compatible string for I2C EEPROM
  ARM: dts: efm32: Add generic compatible string for I2C EEPROM
  ARM: dts: imx: Add generic compatible string for I2C EEPROM
  ARM: dts: keystone: Add generic compatible string for I2C EEPROM
  ARM: dts: lpc18xx: Add generic compatible string for I2C EEPROM
  ARM: dts: r7s72100: Add generic compatible string for I2C EEPROM
  ARM: dts: koelsch: Add generic compatible string for I2C EEPROM
  ARM: dts: socfpga: Add generic compatible string for I2C EEPROM
  ARM: dts: uniphier: Add generic compatible string for I2C EEPROM
  ARM: dts: zynq: Add generic compatibl

Re: [PATCH v4 00/20] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Geert,

On Mon, May 22, 2017 at 9:30 PM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> Hi Javier,
>
> On Mon, May 22, 2017 at 7:15 PM, Javier Martinez Canillas
> <jav...@dowhile0.org> wrote:
>>>> I also wonder why this is really needed if AFAIU "renesas,24c02" is
>>>> compatible with "atmel,24c02". IOW, the driver doesn't need to
>>>> differentiate between the two since the devices are the same and will
>>>> always match using "atmel,24c02".
>>>
>>> It is needed, so that when a difference is found, it can be handled
>>> without updating the DT.
>>
>> Yes, I understand this. What I tried to ask is if there could really
>> be a difference for the same chip type between different vendors, or
>> is just that people were using other manufacturers in the compatible
>> string as a consequence of the DT binding doc and the I2C core
>> ignoring the vendor prefix.
>
> The devices from different vendors are not the same. They contain FLASH
> ROM of a specific size, and glue logic to expose an i2c slave
> interface providing
> an AT24-compatible command set.  They should behave similar within
> the limits of the AT24 "spec".  But the actual implementation may be 
> different.
>

I see, really appreciate your explanation. I'm not familiar with these
devices and driver but the patch-series are needed in order to make
sure that no regressions will happen once the I2C core reports a
proper OF modalias.

>> I don't mind though, I will leave the manufacturers that are different
>> than the atmel variants in the mainline DTS as you and Geert asked.
>
> OK, thanks!
>

Thanks a lot for your feedback!

> Gr{oetje,eeting}s,
>
> Geert
>
> --

Best regards,
Javier

Re: [PATCH v4 00/20] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Rob,

Thanks a lot for your feedback.

On Mon, May 22, 2017 at 6:52 PM, Rob Herring <r...@kernel.org> wrote:
> On Mon, May 22, 2017 at 9:46 AM, Javier Martinez Canillas
> <jav...@dowhile0.org> wrote:

[snip]

>>>> | >
>>>> | > at24@50 {
>>>> | > -   compatible = "at24,24c02";
>>>> | > +   compatible = "at24,24c02", "atmel,24c02";
>>>> |
>>>> | I think you can just drop the at24 compatibles. A new kernel doesn't
>>>> | need it. An old kernel ignores the manufacturer. I checked that u-boot
>>>> | only matches on "atmel,*", so okay there. Don't know about the *BSDs. I
>>>> | couldn't find anything.
>>>>
>>>> I think you misunderstood what Rob means.
>>>>
>>>> In the case above it makes sense to drop the first compatible, as "at24" is
>>>> not a manufacturer, but refers to ATMEL's "AT24" line of i2c FLASH ROMs.
>>>>
>>>> However, in cases where a real vendor/part combo is specified, like on
>>>> r8a7791-koelsch.dts:
>>>>
>>>> -   compatible = "renesas,24c02";
>>>> +   compatible = "atmel,24c02";
>>>>
>>>> you do want to keep the real vendor/part combo, i.e.
>>>>
>>>>  compatible = "renesas,24c02", "atmel,24c02";
>>>
>>> Yes, Geert is correct.
>>>
>>
>> Sorry for misunderstanding your previous comment...
>>
>> How this should be documented in the DT binding? Should I include
>> "renesas" as a valid manufacturer or only list the used
>> <vendor,device> pairs (i.e: "renesas,24c02")?
>
> However you are handling any of the vendors. I'll have to go look.
>

The current DT binding is quite lax when describing this. From
Documentation/devicetree/bindings/eeprom/eeprom.txt:

--
EEPROMs (I2C)

Required properties:

  - compatible : should be ",", like these:

"atmel,24c00", "atmel,24c01", "atmel,24c02", "atmel,24c04",
"atmel,24c08", "atmel,24c16", "atmel,24c32", "atmel,24c64",
"atmel,24c128", "atmel,24c256", "atmel,24c512", "atmel,24c1024"

"catalyst,24c32"

"microchip,24c128"

"ramtron,24c64"

"renesas,r1ex24002"

If there is no specific driver for , a generic
driver based on  is selected. Possible types are:
"24c00", "24c01", "24c02", "24c04", "24c08", "24c16", "24c32", "24c64",
"24c128", "24c256", "24c512", "24c1024", "spd"

  - reg : the I2C address of the EEPROM
--

I think though that this is one of the cases where the Linux I2C
subsystem matching logic is leaking into the DT binding doc, since the
manufacturer prefix is ignored by the I2C core (the I2C device ID
table is used to match and to report a MODALIAS).

But I'll keep the description generic as it is now and only mention
the atmel variants (at and at24) as deprecated then.

>> I also wonder why this is really needed if AFAIU "renesas,24c02" is
>> compatible with "atmel,24c02". IOW, the driver doesn't need to
>> differentiate between the two since the devices are the same and will
>> always match using "atmel,24c02".
>
> It is needed, so that when a difference is found, it can be handled
> without updating the DT.
>

Yes, I understand this. What I tried to ask is if there could really
be a difference for the same chip type between different vendors, or
is just that people were using other manufacturers in the compatible
string as a consequence of the DT binding doc and the I2C core
ignoring the vendor prefix.

I don't mind though, I will leave the manufacturers that are different
than the atmel variants in the mainline DTS as you and Geert asked.

> Rob

Best regards,
Javier

Re: [PATCH v4 00/20] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Geert and Rob,

On Mon, May 22, 2017 at 4:26 PM, Rob Herring <r...@kernel.org> wrote:
> On Mon, May 22, 2017 at 9:23 AM, Geert Uytterhoeven
> <ge...@linux-m68k.org> wrote:
>> Hi Javier,
>>
>> On Mon, May 22, 2017 at 4:01 PM, Javier Martinez Canillas
>> <jav...@dowhile0.org> wrote:
>>> This series is a follow-up to patch [0] that added an OF device ID table
>>> to the at24 EEPROM driver. As you suggested [1], this version instead of
>>> adding entries for every used <vendor,device> tuple, only adds a single
>>> entry for each chip type using the "atmel" vendor as a generic fallback.
>>>
>>> This is a re-spin that addresses some issues pointed out by Rob Herring.
>>>
>>> The first patch documents in the DT binding what's the correct vendor to
>>> use and what are the ones that are being deprecated. The second one adds
>>> the OF device ID table for the at24 driver and the next patches use this
>>> vendor in the compatible string to each DTS that defines a compatible I2C
>>> EEPROM device node.
>>>
>>> Patches can be applied independently since the DTS changes without driver
>>> changes are no-op and the OF table won't be used without the DTS changes.
>>>
>>> [0]: https://lkml.org/lkml/2017/3/14/589
>>> [1]: https://lkml.org/lkml/2017/3/15/99
>>>
>>> Best regards,
>>> Javier
>>>
>>> Changes in v4:
>>> - Document the manufacturers that have been deprecated (Rob Herring).
>>> - Only use the atmel manufacturer in the compatible string instead of
>>>   keeping the deprecated ones (Rob Herring).
>>
>> I think you're referring to this (https://lkml.org/lkml/2017/4/19/1136)?
>>
>> | > --- a/arch/arm/boot/dts/am335x-baltos.dtsi
>> | > +++ b/arch/arm/boot/dts/am335x-baltos.dtsi
>> | > @@ -255,7 +255,7 @@
>> | > };
>> | >
>> | > at24@50 {
>> | > -   compatible = "at24,24c02";
>> | > +   compatible = "at24,24c02", "atmel,24c02";
>> |
>> | I think you can just drop the at24 compatibles. A new kernel doesn't
>> | need it. An old kernel ignores the manufacturer. I checked that u-boot
>> | only matches on "atmel,*", so okay there. Don't know about the *BSDs. I
>> | couldn't find anything.
>>
>> I think you misunderstood what Rob means.
>>
>> In the case above it makes sense to drop the first compatible, as "at24" is
>> not a manufacturer, but refers to ATMEL's "AT24" line of i2c FLASH ROMs.
>>
>> However, in cases where a real vendor/part combo is specified, like on
>> r8a7791-koelsch.dts:
>>
>> -   compatible = "renesas,24c02";
>> +   compatible = "atmel,24c02";
>>
>> you do want to keep the real vendor/part combo, i.e.
>>
>>  compatible = "renesas,24c02", "atmel,24c02";
>
> Yes, Geert is correct.
>

Sorry for misunderstanding your previous comment...

How this should be documented in the DT binding? Should I include
"renesas" as a valid manufacturer or only list the used
<vendor,device> pairs (i.e: "renesas,24c02")?

I also wonder why this is really needed if AFAIU "renesas,24c02" is
compatible with "atmel,24c02". IOW, the driver doesn't need to
differentiate between the two since the devices are the same and will
always match using "atmel,24c02".

> Rob

Best regards,
Javier

[PATCH v4 20/20] powerpc/44x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/warp.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/warp.dts b/arch/powerpc/boot/dts/warp.dts
index e576ee85c42f..ea9053ef4819 100644
--- a/arch/powerpc/boot/dts/warp.dts
+++ b/arch/powerpc/boot/dts/warp.dts
@@ -238,7 +238,7 @@
 
/* This will create 52 and 53 */
at24@52 {
-   compatible = "at,24c04";
+   compatible = "atmel,24c04";
reg = <0x52>;
};
};
-- 
2.9.3

[PATCH v4 19/20] powerpc/83xx: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc8308_p1m.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8349emitx.dts | 4 ++--
 arch/powerpc/boot/dts/mpc8377_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8377_wlan.dts | 2 +-
 arch/powerpc/boot/dts/mpc8378_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8379_rdb.dts  | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts 
b/arch/powerpc/boot/dts/mpc8308_p1m.dts
index 57f86cdf9f36..702ab4fc5b4a 100644
--- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
+++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
@@ -123,7 +123,7 @@
interrupt-parent = <>;
dfsrr;
fram@50 {
-   compatible = "ramtron,24c64";
+   compatible = "atmel,24c64";
reg = <0x50>;
};
};
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 90aed3ac2f69..f49d1cffd927 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -92,7 +92,7 @@
dfsrr;
 
eeprom: at24@50 {
-   compatible = "st,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
@@ -130,7 +130,7 @@
};
 
spd: at24@51 {
-   compatible = "at24,spd";
+   compatible = "atmel,spd";
reg = <0x51>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_rdb.dts 
b/arch/powerpc/boot/dts/mpc8377_rdb.dts
index e32613963ab0..5e85d8c93bca 100644
--- a/arch/powerpc/boot/dts/mpc8377_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8377_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_wlan.dts 
b/arch/powerpc/boot/dts/mpc8377_wlan.dts
index c0c790168b96..fee15fcbb46f 100644
--- a/arch/powerpc/boot/dts/mpc8377_wlan.dts
+++ b/arch/powerpc/boot/dts/mpc8377_wlan.dts
@@ -135,7 +135,7 @@
dfsrr;
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8378_rdb.dts 
b/arch/powerpc/boot/dts/mpc8378_rdb.dts
index 71842fcd621f..e973d61956b9 100644
--- a/arch/powerpc/boot/dts/mpc8378_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8378_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8379_rdb.dts 
b/arch/powerpc/boot/dts/mpc8379_rdb.dts
index e442a29b2fe0..ed5d12ff2ee0 100644
--- a/arch/powerpc/boot/dts/mpc8379_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8379_rdb.dts
@@ -148,7 +148,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v4 18/20] powerpc/512x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc5121ads.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts 
b/arch/powerpc/boot/dts/mpc5121ads.dts
index 75888ce2c792..fcaa9bad4bda 100644
--- a/arch/powerpc/boot/dts/mpc5121ads.dts
+++ b/arch/powerpc/boot/dts/mpc5121ads.dts
@@ -94,7 +94,7 @@
};
 
eeprom@50 {
-   compatible = "at,24c32";
+   compatible = "atmel,24c32";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v4 17/20] powerpc/fsl: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/fsl/b4qds.dtsi|  8 
 arch/powerpc/boot/dts/fsl/c293pcie.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p1010rdb.dtsi |  2 +-
 arch/powerpc/boot/dts/fsl/p1023rdb.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p2041rdb.dts  |  4 ++--
 arch/powerpc/boot/dts/fsl/p3041ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p4080ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5020ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5040ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/t208xqds.dtsi |  8 
 arch/powerpc/boot/dts/fsl/t4240qds.dts  | 12 ++--
 arch/powerpc/boot/dts/fsl/t4240rdb.dts  |  6 +++---
 12 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/b4qds.dtsi 
b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
index 3785ef826d07..999efd3bc167 100644
--- a/arch/powerpc/boot/dts/fsl/b4qds.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
@@ -166,19 +166,19 @@
reg = <0>;
 
eeprom@50 {
-   compatible = "at24,24c64";
+   compatible = "atmel,24c64";
reg = <0x50>;
};
eeprom@51 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x51>;
};
eeprom@53 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x53>;
};
eeprom@57 {
-   compatible = "at24,24c256";
+   compatible = "atmel,24c256";
reg = <0x57>;
};
rtc@68 {
diff --git a/arch/powerpc/boot/dts/fsl/c293pcie.dts 
b/arch/powerpc/boot/dts/fsl/c293pcie.dts
index 66709788429d..c39b007b6f2c 100644
--- a/arch/powerpc/boot/dts/fsl/c293pcie.dts
+++ b/arch/powerpc/boot/dts/fsl/c293pcie.dts
@@ -153,7 +153,7 @@
  {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c1024";
+   compatible = "atmel,24c1024";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi 
b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
index a8e4ba070104..56e69a3a4eb9 100644
--- a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
@@ -89,7 +89,7 @@
 _soc {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c256";
+   compatible = "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1023rdb.dts 
b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
index 9716ca64651c..ead928364beb 100644
--- a/arch/powerpc/boot/dts/fsl/p1023rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
@@ -79,7 +79,7 @@
 
i2c@3000 {
eeprom@53 {
-   compatible = "at24,24c04";
+   compatible = "atmel,24c04";
reg = <0x53>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p2041rdb.dts 
b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
index e50fea95a853..950816b9d6e1 100644
--- a/arch/powerpc/boot/dts/fsl/p2041rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
@@ -127,7 +127,7 @@
   

[PATCH v4 16/20] powerpc/5200: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@dowhile0.org>

---

Changes in v4:
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/digsy_mtc.dts | 2 +-
 arch/powerpc/boot/dts/pcm030.dts| 2 +-
 arch/powerpc/boot/dts/pcm032.dts| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/boot/dts/digsy_mtc.dts 
b/arch/powerpc/boot/dts/digsy_mtc.dts
index 955bff629df3..c280e75c86bf 100644
--- a/arch/powerpc/boot/dts/digsy_mtc.dts
+++ b/arch/powerpc/boot/dts/digsy_mtc.dts
@@ -73,7 +73,7 @@
 
i2c@3d00 {
eeprom@50 {
-   compatible = "at,24c08";
+   compatible = "atmel,24c08";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 192e66af0001..dd6b8002d716 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -71,7 +71,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm032.dts
index 96b139bf50e9..d77bf80e2847 100644
--- a/arch/powerpc/boot/dts/pcm032.dts
+++ b/arch/powerpc/boot/dts/pcm032.dts
@@ -75,7 +75,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
-- 
2.9.3

[PATCH v4 00/20] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

This series is a follow-up to patch [0] that added an OF device ID table
to the at24 EEPROM driver. As you suggested [1], this version instead of
adding entries for every used <vendor,device> tuple, only adds a single
entry for each chip type using the "atmel" vendor as a generic fallback.

This is a re-spin that addresses some issues pointed out by Rob Herring.

The first patch documents in the DT binding what's the correct vendor to
use and what are the ones that are being deprecated. The second one adds
the OF device ID table for the at24 driver and the next patches use this
vendor in the compatible string to each DTS that defines a compatible I2C
EEPROM device node.

Patches can be applied independently since the DTS changes without driver
changes are no-op and the OF table won't be used without the DTS changes.

[0]: https://lkml.org/lkml/2017/3/14/589
[1]: https://lkml.org/lkml/2017/3/15/99

Best regards,
Javier

Changes in v4:
- Document the manufacturers that have been deprecated (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).
- Only use the atmel manufacturer in the compatible string instead of
  keeping the deprecated ones (Rob Herring).

Changes in v3:
- Fix wrong .data values for "atmel,24c02" and "atmel,24c64" entries.
- Add Geert Uytterhoeven reviewed-by tag.
- Add Geert Uytterhoeven reviewed-by tag.

Changes in v2:
- Only add a single OF device ID entry for each device type (Wolfram Sang).

Javier Martinez Canillas (20):
  dt-bindings: i2c: eeprom: Document vendor to be used and deprecated
ones
  eeprom: at24: Add OF device ID table
  ARM: dts: omap: Add generic compatible string for I2C EEPROM
  ARM: dts: turris-omnia: Add generic compatible string for I2C EEPROM
  ARM: dts: efm32: Add generic compatible string for I2C EEPROM
  ARM: dts: imx: Add generic compatible string for I2C EEPROM
  ARM: dts: keystone: Add generic compatible string for I2C EEPROM
  ARM: dts: lpc18xx: Add generic compatible string for I2C EEPROM
  ARM: dts: r7s72100: Add generic compatible string for I2C EEPROM
  ARM: dts: koelsch: Add generic compatible string for I2C EEPROM
  ARM: dts: socfpga: Add generic compatible string for I2C EEPROM
  ARM: dts: uniphier: Add generic compatible string for I2C EEPROM
  ARM: dts: zynq: Add generic compatible string for I2C EEPROM
  arm64: dts: ls1043a: Add generic compatible string for I2C EEPROM
  arm64: zynqmp: Add generic compatible string for I2C EEPROM
  powerpc/5200: Add generic compatible string for I2C EEPROM
  powerpc/fsl: Add generic compatible string for I2C EEPROM
  powerpc/512x: Add generic compatible string for I2C EEPROM
  powerpc/83xx: Add generic compatible string for I2C EEPROM
  powerpc/44x: Add generic compatible string for I2C EEPROM

 .../devicetree/bindings/eeprom/eeprom.txt  | 14 ++---
 arch/arm/boot/dts/am335x-baltos.dtsi   |  2 +-
 arch/arm/boot/dts/am335x-base0033.dts  |  2 +-
 arch/arm/boot/dts/am335x-bone-common.dtsi  | 10 ++--
 arch/arm/boot/dts/am335x-nano.dts

[PATCH v3 21/21] powerpc/44x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/warp.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/warp.dts b/arch/powerpc/boot/dts/warp.dts
index e576ee85c42f..19e0ec83b1a5 100644
--- a/arch/powerpc/boot/dts/warp.dts
+++ b/arch/powerpc/boot/dts/warp.dts
@@ -238,7 +238,7 @@
 
/* This will create 52 and 53 */
at24@52 {
-   compatible = "at,24c04";
+   compatible = "at,24c04", "atmel,24c04";
reg = <0x52>;
};
};
-- 
2.9.3

[PATCH v3 20/21] powerpc/83xx: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc8308_p1m.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8349emitx.dts | 2 +-
 arch/powerpc/boot/dts/mpc8377_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8377_wlan.dts | 2 +-
 arch/powerpc/boot/dts/mpc8378_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8379_rdb.dts  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts 
b/arch/powerpc/boot/dts/mpc8308_p1m.dts
index 57f86cdf9f36..cab933b3957a 100644
--- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
+++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
@@ -123,7 +123,7 @@
interrupt-parent = <>;
dfsrr;
fram@50 {
-   compatible = "ramtron,24c64";
+   compatible = "ramtron,24c64", "atmel,24c64";
reg = <0x50>;
};
};
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 90aed3ac2f69..00c5ef08e474 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -92,7 +92,7 @@
dfsrr;
 
eeprom: at24@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256", "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_rdb.dts 
b/arch/powerpc/boot/dts/mpc8377_rdb.dts
index e32613963ab0..02c59d05fe16 100644
--- a/arch/powerpc/boot/dts/mpc8377_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8377_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "at24,24c256", 
"atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_wlan.dts 
b/arch/powerpc/boot/dts/mpc8377_wlan.dts
index c0c790168b96..2bc3ed8a18c3 100644
--- a/arch/powerpc/boot/dts/mpc8377_wlan.dts
+++ b/arch/powerpc/boot/dts/mpc8377_wlan.dts
@@ -135,7 +135,7 @@
dfsrr;
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "at24,24c256", 
"atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8378_rdb.dts 
b/arch/powerpc/boot/dts/mpc8378_rdb.dts
index 71842fcd621f..9e681e3d6064 100644
--- a/arch/powerpc/boot/dts/mpc8378_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8378_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "at24,24c256", 
"atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8379_rdb.dts 
b/arch/powerpc/boot/dts/mpc8379_rdb.dts
index e442a29b2fe0..3197bb78e19b 100644
--- a/arch/powerpc/boot/dts/mpc8379_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8379_rdb.dts
@@ -148,7 +148,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = "at24,24c256", 
"atmel,24c256";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v3 19/21] powerpc/512x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/mpc5121ads.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts 
b/arch/powerpc/boot/dts/mpc5121ads.dts
index 75888ce2c792..ece5d60256ee 100644
--- a/arch/powerpc/boot/dts/mpc5121ads.dts
+++ b/arch/powerpc/boot/dts/mpc5121ads.dts
@@ -94,7 +94,7 @@
};
 
eeprom@50 {
-   compatible = "at,24c32";
+   compatible = "at,24c32", "atmel,24c32";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v3 18/21] powerpc/fsl: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/fsl/b4qds.dtsi|  8 
 arch/powerpc/boot/dts/fsl/c293pcie.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p1010rdb.dtsi |  2 +-
 arch/powerpc/boot/dts/fsl/p1023rdb.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p2041rdb.dts  |  4 ++--
 arch/powerpc/boot/dts/fsl/p3041ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p4080ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5020ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5040ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/t208xqds.dtsi |  8 
 arch/powerpc/boot/dts/fsl/t4240qds.dts  | 12 ++--
 arch/powerpc/boot/dts/fsl/t4240rdb.dts  |  6 +++---
 12 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/b4qds.dtsi 
b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
index 3785ef826d07..18053d59be01 100644
--- a/arch/powerpc/boot/dts/fsl/b4qds.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
@@ -166,19 +166,19 @@
reg = <0>;
 
eeprom@50 {
-   compatible = "at24,24c64";
+   compatible = "at24,24c64", 
"atmel,24c64";
reg = <0x50>;
};
eeprom@51 {
-   compatible = "at24,24c256";
+   compatible = "at24,24c256", 
"atmel,24c256";
reg = <0x51>;
};
eeprom@53 {
-   compatible = "at24,24c256";
+   compatible = "at24,24c256", 
"atmel,24c256";
reg = <0x53>;
};
eeprom@57 {
-   compatible = "at24,24c256";
+   compatible = "at24,24c256", 
"atmel,24c256";
reg = <0x57>;
};
rtc@68 {
diff --git a/arch/powerpc/boot/dts/fsl/c293pcie.dts 
b/arch/powerpc/boot/dts/fsl/c293pcie.dts
index 66709788429d..5e905e0857cf 100644
--- a/arch/powerpc/boot/dts/fsl/c293pcie.dts
+++ b/arch/powerpc/boot/dts/fsl/c293pcie.dts
@@ -153,7 +153,7 @@
  {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c1024";
+   compatible = "st,24c1024", "atmel,24c1024";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi 
b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
index a8e4ba070104..2ca9cee2ddeb 100644
--- a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
@@ -89,7 +89,7 @@
 _soc {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256", "atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1023rdb.dts 
b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
index 9716ca64651c..ae38ed66a7a4 100644
--- a/arch/powerpc/boot/dts/fsl/p1023rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
@@ -79,7 +79,7 @@
 
i2c@3000 {
eeprom@53 {
-   compatible = "at24,24c04";
+   compatible = "at24,24c04", "atmel,24c04";
reg = <0x53>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p2041rdb.dts 
b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
index e50fea95a853..0c34e8c46961 100644
--- a/arch/powerpc/boot/dts/fsl/p2041rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
@@ -127,7 +127,7 @@
  

[PATCH v3 17/21] powerpc/5200: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v3: None
Changes in v2: None

 arch/powerpc/boot/dts/digsy_mtc.dts | 2 +-
 arch/powerpc/boot/dts/pcm030.dts| 2 +-
 arch/powerpc/boot/dts/pcm032.dts| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/boot/dts/digsy_mtc.dts 
b/arch/powerpc/boot/dts/digsy_mtc.dts
index 955bff629df3..0acb6ff7fc22 100644
--- a/arch/powerpc/boot/dts/digsy_mtc.dts
+++ b/arch/powerpc/boot/dts/digsy_mtc.dts
@@ -73,7 +73,7 @@
 
i2c@3d00 {
eeprom@50 {
-   compatible = "at,24c08";
+   compatible = "at,24c08", "atmel,24c08";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 192e66af0001..836e47cc4bed 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -71,7 +71,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32", "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm032.dts
index 96b139bf50e9..576249bf2fb9 100644
--- a/arch/powerpc/boot/dts/pcm032.dts
+++ b/arch/powerpc/boot/dts/pcm032.dts
@@ -75,7 +75,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32", "atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
-- 
2.9.3

[PATCH v3 00/21] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

This series is a follow-up to patch [0] that added an OF device ID table
to the at24 EEPROM driver. As you suggested [1], this version instead of
adding entries for every used <vendor,device> tuple, only adds a single
entry for each chip type using the "atmel" vendor as a generic fallback.

The first patch adds the OF device ID table for the at24 driver and the
next patches adds a generic fallback compatible string to each DTS that
defines a compatible I2C EEPROM device node.

Patches can be applied independently since the DTS change without the
driver change is a no-op and the OF device table won't be used without
the DTS changes.

[0]: https://lkml.org/lkml/2017/3/14/589
[1]: https://lkml.org/lkml/2017/3/15/99

Best regards,
Javier

Changes in v3:
- Fix wrong .data values for "atmel,24c02" and "atmel,24c64" entries.
- Add Peter Rosin acked-by tag.
- Add Geert Uytterhoeven reviewed-by tag.
- Add Geert Uytterhoeven reviewed-by tag.

Changes in v2:
- Only add a single OF device ID entry for each device type (Wolfram Sang).

Javier Martinez Canillas (21):
  dt-bindings: i2c: eeprom: Document manufacturer used as generic
fallback
  eeprom: at24: Add OF device ID table
  ARM: dts: omap: Add generic compatible string for I2C EEPROM
  ARM: dts: turris-omnia: Add generic compatible string for I2C EEPROM
  ARM: dts: at91: Add generic compatible string for I2C EEPROM
  ARM: dts: efm32: Add generic compatible string for I2C EEPROM
  ARM: dts: imx: Add generic compatible string for I2C EEPROM
  ARM: dts: keystone: Add generic compatible string for I2C EEPROM
  ARM: dts: lpc18xx: Add generic compatible string for I2C EEPROM
  ARM: dts: r7s72100: Add generic compatible string for I2C EEPROM
  ARM: dts: koelsch: Add generic compatible string for I2C EEPROM
  ARM: dts: socfpga: Add generic compatible string for I2C EEPROM
  ARM: dts: uniphier: Add generic compatible string for I2C EEPROM
  ARM: dts: zynq: Add generic compatible string for I2C EEPROM
  arm64: dts: ls1043a: Add generic compatible string for I2C EEPROM
  arm64: zynqmp: Add generic compatible string for I2C EEPROM
  powerpc/5200: Add generic compatible string for I2C EEPROM
  powerpc/fsl: Add generic compatible string for I2C EEPROM
  powerpc/512x: Add generic compatible string for I2C EEPROM
  powerpc/83xx: Add generic compatible string for I2C EEPROM
  powerpc/44x: Add generic compatible string for I2C EEPROM

 .../devicetree/bindings/eeprom/eeprom.txt  |  3 +-
 arch/arm/boot/dts/am335x-baltos.dtsi   |  2 +-
 arch/arm/boot/dts/am335x-base0033.dts  |  2 +-
 arch/arm/boot/dts/am335x-bone-common.dtsi  | 10 ++--
 arch/arm/boot/dts/am335x-nano.dts  |  2 +-
 arch/arm/boot/dts/am335x-pepper.dts|  2 +-
 arch/arm/boot/dts/am335x-shc.dts   |  2 +-
 arch/arm/boot/dts/am335x-sl50.dts  |  2 +-
 arch/arm/boot/dts/am437x-idk-evm.dts   |  2 +-
 arch/arm/boot/dts/am437x-sk-evm.dts|  2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts   |  2 +-
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi|  2 +-
 arch/arm/boot/dts/armada-385-turris-omnia.dts  |  2 +-
 arch/arm/boot/dts/at91-linea.dtsi  |  2 +-
 arch/arm/boot/dts/at91-tse850-3.dts|  2 +-
 arch/arm/boot/dts/efm32gg-dk3750.dts   |  2 +-
 arch/arm/boot/dts/imx27-phytec-phycard-s-som.dtsi  |  2 +-
 arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi|  2 +-
 arch/arm/boot/dts/imx28-evk.dts|  2 +-
 arch/arm/boot/dts/imx53-tqma53.dtsi|  2 +-
 arch/arm/boot/dts/imx6q-cm-fx6.dts |  2 +-
 arch/arm/boot/dts/imx6q-utilite-pro.dts|  2 +-
 arch/arm/boot/dts/keystone-k2e-evm.dts |  2 +-
 arch/arm/boot/dts/keystone-k2hk-evm.dts|  2 +-
 arch/arm/boot/dts/keystone-k2l-evm.dts |  2 +-
 arch/arm/boot/dts/lpc4337-ciaa.dts |  6 +-
 arch/arm/boot/dts/lpc4350-hitex-eval.dts   |  2 +-
 arch/arm/boot/dts/lpc4357-ea4357-devkit.dts|  2 +-
 arch/arm/boot/dts/omap3-cm-t3x.dtsi|  2 +-
 arch/arm/boot/dts/omap3-gta04.dtsi |  2 +-
 arch/arm/boot/dts/omap3-sb-t35.dtsi|  2 +-
 arch/arm/boot/dts/omap4-var-som-om44.dtsi  |  2 +-
 arch/arm/boot/dts/omap5-cm-t54.dts |  2 +-
 arch/arm/boot/dts/omap5-sbc-t54.dts|  2 +-
 arch/arm/boot/dts/r7s72100-genmai.dts  |  2 +-
 arch/arm/boot/dts/r8a7791-koelsch.dts  |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts |  2 +-
 arch/arm/boot/dts/uniphier-pro4-ace.dts|  2 +-
 arch/arm/boot/dts/uniphier-pro4-sanji.dts  |  2 +-
 arch/arm/boot/dts/uniphier-pxs2-gentil.dts |  2 +-
 arch/arm/boot/dts/zynq-zc702.dts   |  2 +-
 arch/arm/boot/dts/zynq-zc706.dts   |  2 +-
 arch/arm64/bo

[PATCH v2 19/22] powerpc/512x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v2: None

 arch/powerpc/boot/dts/mpc5121ads.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts 
b/arch/powerpc/boot/dts/mpc5121ads.dts
index 75888ce2c792..dd261fd2ee27 100644
--- a/arch/powerpc/boot/dts/mpc5121ads.dts
+++ b/arch/powerpc/boot/dts/mpc5121ads.dts
@@ -94,7 +94,7 @@
};
 
eeprom@50 {
-   compatible = "at,24c32";
+   compatible = "at,24c32","atmel,24c32";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v2 21/22] powerpc/5200: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v2: None

 arch/powerpc/boot/dts/pcm030.dts | 2 +-
 arch/powerpc/boot/dts/pcm032.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 192e66af0001..c7a26c9f634e 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -71,7 +71,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32","atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm032.dts
index 96b139bf50e9..27d6a1e4c018 100644
--- a/arch/powerpc/boot/dts/pcm032.dts
+++ b/arch/powerpc/boot/dts/pcm032.dts
@@ -75,7 +75,7 @@
reg = <0x51>;
};
eeprom@52 {
-   compatible = "catalyst,24c32";
+   compatible = "catalyst,24c32","atmel,24c32";
reg = <0x52>;
pagesize = <32>;
};
-- 
2.9.3

[PATCH v2 17/22] powerpc/5200: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v2: None

 arch/powerpc/boot/dts/digsy_mtc.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/digsy_mtc.dts 
b/arch/powerpc/boot/dts/digsy_mtc.dts
index 955bff629df3..6c804254b885 100644
--- a/arch/powerpc/boot/dts/digsy_mtc.dts
+++ b/arch/powerpc/boot/dts/digsy_mtc.dts
@@ -73,7 +73,7 @@
 
i2c@3d00 {
eeprom@50 {
-   compatible = "at,24c08";
+   compatible = "at,24c08","atmel,24c08";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v2 22/22] powerpc/44x: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

---

Changes in v2: None

 arch/powerpc/boot/dts/warp.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/warp.dts b/arch/powerpc/boot/dts/warp.dts
index e576ee85c42f..5b36dbf6c9f4 100644
--- a/arch/powerpc/boot/dts/warp.dts
+++ b/arch/powerpc/boot/dts/warp.dts
@@ -238,7 +238,7 @@
 
/* This will create 52 and 53 */
at24@52 {
-   compatible = "at,24c04";
+   compatible = "at,24c04","atmel,24c04";
reg = <0x52>;
};
};
-- 
2.9.3

[PATCH v2 20/22] powerpc/83xx: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v2: None

 arch/powerpc/boot/dts/mpc8308_p1m.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8349emitx.dts | 2 +-
 arch/powerpc/boot/dts/mpc8377_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8377_wlan.dts | 2 +-
 arch/powerpc/boot/dts/mpc8378_rdb.dts  | 2 +-
 arch/powerpc/boot/dts/mpc8379_rdb.dts  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts 
b/arch/powerpc/boot/dts/mpc8308_p1m.dts
index 57f86cdf9f36..f7803bec2873 100644
--- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
+++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
@@ -123,7 +123,7 @@
interrupt-parent = <>;
dfsrr;
fram@50 {
-   compatible = "ramtron,24c64";
+   compatible = "ramtron,24c64","atmel,24c64";
reg = <0x50>;
};
};
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 90aed3ac2f69..7f759b9b9dc9 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -92,7 +92,7 @@
dfsrr;
 
eeprom: at24@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256","atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_rdb.dts 
b/arch/powerpc/boot/dts/mpc8377_rdb.dts
index e32613963ab0..0f8c53c69f83 100644
--- a/arch/powerpc/boot/dts/mpc8377_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8377_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = 
"at24,24c256","atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8377_wlan.dts 
b/arch/powerpc/boot/dts/mpc8377_wlan.dts
index c0c790168b96..1f04b4f99c8f 100644
--- a/arch/powerpc/boot/dts/mpc8377_wlan.dts
+++ b/arch/powerpc/boot/dts/mpc8377_wlan.dts
@@ -135,7 +135,7 @@
dfsrr;
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = 
"at24,24c256","atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8378_rdb.dts 
b/arch/powerpc/boot/dts/mpc8378_rdb.dts
index 71842fcd621f..0f73f239370f 100644
--- a/arch/powerpc/boot/dts/mpc8378_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8378_rdb.dts
@@ -150,7 +150,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = 
"at24,24c256","atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/mpc8379_rdb.dts 
b/arch/powerpc/boot/dts/mpc8379_rdb.dts
index e442a29b2fe0..d7ee3089a0f9 100644
--- a/arch/powerpc/boot/dts/mpc8379_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8379_rdb.dts
@@ -148,7 +148,7 @@
};
 
at24@50 {
-   compatible = "at24,24c256";
+   compatible = 
"at24,24c256","atmel,24c256";
reg = <0x50>;
};
 
-- 
2.9.3

[PATCH v2 18/22] powerpc/fsl: Add generic compatible string for I2C EEPROM

[Javier Martinez Canillas]

The at24 driver allows to register I2C EEPROM chips using different vendor
and devices, but the I2C subsystem does not take the vendor into account
when matching using the I2C table since it only has device entries.

But when matching using an OF table, both the vendor and device has to be
taken into account so the driver defines only a set of compatible strings
using the "atmel" vendor as a generic fallback for compatible I2C devices.

So add this generic fallback to the device node compatible string to make
the device to match the driver using the OF device ID table.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

Changes in v2: None

 arch/powerpc/boot/dts/fsl/b4qds.dtsi|  8 
 arch/powerpc/boot/dts/fsl/c293pcie.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p1010rdb.dtsi |  2 +-
 arch/powerpc/boot/dts/fsl/p1023rdb.dts  |  2 +-
 arch/powerpc/boot/dts/fsl/p2041rdb.dts  |  4 ++--
 arch/powerpc/boot/dts/fsl/p3041ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p4080ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5020ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/p5040ds.dts   |  4 ++--
 arch/powerpc/boot/dts/fsl/t208xqds.dtsi |  8 
 arch/powerpc/boot/dts/fsl/t4240qds.dts  | 12 ++--
 arch/powerpc/boot/dts/fsl/t4240rdb.dts  |  6 +++---
 12 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/b4qds.dtsi 
b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
index 3785ef826d07..2813e8bb5d1e 100644
--- a/arch/powerpc/boot/dts/fsl/b4qds.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4qds.dtsi
@@ -166,19 +166,19 @@
reg = <0>;
 
eeprom@50 {
-   compatible = "at24,24c64";
+   compatible = 
"at24,24c64","atmel,24c64";
reg = <0x50>;
};
eeprom@51 {
-   compatible = "at24,24c256";
+   compatible = 
"at24,24c256","atmel,24c256";
reg = <0x51>;
};
eeprom@53 {
-   compatible = "at24,24c256";
+   compatible = 
"at24,24c256","atmel,24c256";
reg = <0x53>;
};
eeprom@57 {
-   compatible = "at24,24c256";
+   compatible = 
"at24,24c256","atmel,24c256";
reg = <0x57>;
};
rtc@68 {
diff --git a/arch/powerpc/boot/dts/fsl/c293pcie.dts 
b/arch/powerpc/boot/dts/fsl/c293pcie.dts
index 66709788429d..2b931b1b5767 100644
--- a/arch/powerpc/boot/dts/fsl/c293pcie.dts
+++ b/arch/powerpc/boot/dts/fsl/c293pcie.dts
@@ -153,7 +153,7 @@
  {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c1024";
+   compatible = "st,24c1024","atmel,24c1024";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi 
b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
index a8e4ba070104..868da24bdbf1 100644
--- a/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010rdb.dtsi
@@ -89,7 +89,7 @@
 _soc {
i2c@3000 {
eeprom@50 {
-   compatible = "st,24c256";
+   compatible = "st,24c256","atmel,24c256";
reg = <0x50>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p1023rdb.dts 
b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
index 9716ca64651c..241988d48290 100644
--- a/arch/powerpc/boot/dts/fsl/p1023rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p1023rdb.dts
@@ -79,7 +79,7 @@
 
i2c@3000 {
eeprom@53 {
-   compatible = "at24,24c04";
+   compatible = "at24,24c04","atmel,24c04";
reg = <0x53>;
};
 
diff --git a/arch/powerpc/boot/dts/fsl/p2041rdb.dts 
b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
index e50fea95a853..65b004ff2789 100644
--- a/arch/powerpc/boot/dts/fsl/p2041rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/p2041rdb.dts
@@ -127,7 +127,7 @@
 

[PATCH v2 00/22] eeprom: at24: Add OF device ID table

[Javier Martinez Canillas]

Hello Wolfram,

This series is a follow-up to patch [0] that added an OF device ID table
to the at24 EEPROM driver. As you suggested [1], this version instead of
adding entries for every used <vendor,device> tuple, only adds a single
entry for each chip type using the "atmel" vendor as a generic fallback.

The first patch adds the OF device ID table for the at24 driver and the
next patches adds a generic fallback compatible string to each DTS that
defines a compatible I2C EEPROM device node.

Patches can be applied independently since the DTS change without the
driver change is a no-op and the OF device table won't be used without
the DTS changes.

[0]: https://lkml.org/lkml/2017/3/14/589
[1]: https://lkml.org/lkml/2017/3/15/99

Best regards,
Javier

Changes in v2:
- Only add a single OF device ID entry for each device type (Wolfram Sang).

Javier Martinez Canillas (22):
  dt-bindings: i2c: eeprom: Document manufacturer used as generic
fallback
  eeprom: at24: Add OF device ID table
  ARM: dts: omap: Add generic compatible string for I2C EEPROM
  ARM: dts: turris-omnia: Add generic compatible string for I2C EEPROM
  ARM: dts: at91: Add generic compatible string for I2C EEPROM
  ARM: dts: efm32: Add generic compatible string for I2C EEPROM
  ARM: dts: imx: Add generic compatible string for I2C EEPROM
  ARM: dts: keystone: Add generic compatible string for I2C EEPROM
  ARM: dts: lpc18xx: Add generic compatible string for I2C EEPROM
  ARM: dts: r7s72100: Add generic compatible string for I2C EEPROM
  ARM: dts: koelsch: Add generic compatible string for I2C EEPROM
  ARM: dts: socfpga: Add generic compatible string for I2C EEPROM
  ARM: dts: uniphier: Add generic compatible string for I2C EEPROM
  ARM: dts: zynq: Add generic compatible string for I2C EEPROM
  arm64: dts: ls1043a: Add generic compatible string for I2C EEPROM
  arm64: zynqmp: Add generic compatible string for I2C EEPROM
  powerpc/5200: Add generic compatible string for I2C EEPROM
  powerpc/fsl: Add generic compatible string for I2C EEPROM
  powerpc/512x: Add generic compatible string for I2C EEPROM
  powerpc/83xx: Add generic compatible string for I2C EEPROM
  powerpc/5200: Add generic compatible string for I2C EEPROM
  powerpc/44x: Add generic compatible string for I2C EEPROM

 .../devicetree/bindings/eeprom/eeprom.txt  |  3 +-
 arch/arm/boot/dts/am335x-baltos.dtsi   |  2 +-
 arch/arm/boot/dts/am335x-base0033.dts  |  2 +-
 arch/arm/boot/dts/am335x-bone-common.dtsi  | 10 ++--
 arch/arm/boot/dts/am335x-nano.dts  |  2 +-
 arch/arm/boot/dts/am335x-pepper.dts|  2 +-
 arch/arm/boot/dts/am335x-shc.dts   |  2 +-
 arch/arm/boot/dts/am335x-sl50.dts  |  2 +-
 arch/arm/boot/dts/am437x-idk-evm.dts   |  2 +-
 arch/arm/boot/dts/am437x-sk-evm.dts|  2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts   |  2 +-
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi|  2 +-
 arch/arm/boot/dts/armada-385-turris-omnia.dts  |  2 +-
 arch/arm/boot/dts/at91-linea.dtsi  |  2 +-
 arch/arm/boot/dts/at91-tse850-3.dts|  2 +-
 arch/arm/boot/dts/efm32gg-dk3750.dts   |  2 +-
 arch/arm/boot/dts/imx27-phytec-phycard-s-som.dtsi  |  2 +-
 arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi|  2 +-
 arch/arm/boot/dts/imx28-evk.dts|  2 +-
 arch/arm/boot/dts/imx53-tqma53.dtsi|  2 +-
 arch/arm/boot/dts/imx6q-cm-fx6.dts |  2 +-
 arch/arm/boot/dts/imx6q-utilite-pro.dts|  2 +-
 arch/arm/boot/dts/keystone-k2e-evm.dts |  2 +-
 arch/arm/boot/dts/keystone-k2hk-evm.dts|  2 +-
 arch/arm/boot/dts/keystone-k2l-evm.dts |  2 +-
 arch/arm/boot/dts/lpc4337-ciaa.dts |  6 +-
 arch/arm/boot/dts/lpc4350-hitex-eval.dts   |  2 +-
 arch/arm/boot/dts/lpc4357-ea4357-devkit.dts|  2 +-
 arch/arm/boot/dts/omap3-cm-t3x.dtsi|  2 +-
 arch/arm/boot/dts/omap3-gta04.dtsi |  2 +-
 arch/arm/boot/dts/omap3-sb-t35.dtsi|  2 +-
 arch/arm/boot/dts/omap4-var-som-om44.dtsi  |  2 +-
 arch/arm/boot/dts/omap5-cm-t54.dts |  2 +-
 arch/arm/boot/dts/omap5-sbc-t54.dts|  2 +-
 arch/arm/boot/dts/r7s72100-genmai.dts  |  2 +-
 arch/arm/boot/dts/r8a7791-koelsch.dts  |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts |  2 +-
 arch/arm/boot/dts/uniphier-pro4-ace.dts|  2 +-
 arch/arm/boot/dts/uniphier-pro4-sanji.dts  |  2 +-
 arch/arm/boot/dts/uniphier-pxs2-gentil.dts |  2 +-
 arch/arm/boot/dts/zynq-zc702.dts   |  2 +-
 arch/arm/boot/dts/zynq-zc706.dts   |  2 +-
 arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts  |  4 +-
 arch/arm64/boot/dts/xilinx/zynqmp-ep108.dts|  4 +-
 arch/powerpc/boot/dts/digsy_mtc.dts|  2 

Re: [05/27] macintosh: therm_windtunnel: Export I2C module alias information

[Javier Martinez Canillas]

Hello Michael,

On 08/19/2015 02:51 AM, Michael Ellerman wrote:
 On Tue, 2015-08-18 at 12:35 +0200, Javier Martinez Canillas wrote:
 Hello Michael,

 On 08/18/2015 12:24 PM, Michael Ellerman wrote:
 On Thu, 2015-30-07 at 16:18:30 UTC, Javier Martinez Canillas wrote:
 The I2C core always reports the MODALIAS uevent as i2c:client name
 regardless if the driver was matched using the I2C id_table or the
 of_match_table. So the driver needs to export the I2C table and this
 be built into the module or udev won't have the necessary information
 to auto load the correct module when the device is added.

 Signed-off-by: Javier Martinez Canillas jav...@osg.samsung.com
 ---

  drivers/macintosh/therm_windtunnel.c | 1 +
  1 file changed, 1 insertion(+)

 Who are you expecting to merge this?


 I was expecting Benjamin Herrenschmidt since he is listed in MAINTAINERS
 for drivers/macintosh. I cc'ed him in the patch but now in your answer I
 don't see him in the cc list, strange.
 
 That's the mailing list dropping him from CC because he's subscribed.
 
 But I'll be happy to re-post if there is another person who is handling
 the patches for this driver now.

 BTW there is another patch [0] for the same driver to export the OF id
 table information, that was not picked either.
 
 Yep, I'll grab them both.


Perfect, thanks a lot!
 
 cheers
 
 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [05/27] macintosh: therm_windtunnel: Export I2C module alias information

[Javier Martinez Canillas]

Hello Michael,

On 08/18/2015 12:24 PM, Michael Ellerman wrote:
 On Thu, 2015-30-07 at 16:18:30 UTC, Javier Martinez Canillas wrote:
 The I2C core always reports the MODALIAS uevent as i2c:client name
 regardless if the driver was matched using the I2C id_table or the
 of_match_table. So the driver needs to export the I2C table and this
 be built into the module or udev won't have the necessary information
 to auto load the correct module when the device is added.

 Signed-off-by: Javier Martinez Canillas jav...@osg.samsung.com
 ---

  drivers/macintosh/therm_windtunnel.c | 1 +
  1 file changed, 1 insertion(+)
 
 Who are you expecting to merge this?
 

I was expecting Benjamin Herrenschmidt since he is listed in MAINTAINERS
for drivers/macintosh. I cc'ed him in the patch but now in your answer I
don't see him in the cc list, strange.

But I'll be happy to re-post if there is another person who is handling
the patches for this driver now.

BTW there is another patch [0] for the same driver to export the OF id
table information, that was not picked either.

 cheers
 

[0]: https://lkml.org/lkml/2015/7/30/503

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev