Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate

[Florian Fainelli]

On 5/7/24 04:44, Arnd Bergmann wrote:

On Tue, May 7, 2024, at 13:10, Daniel Vetter wrote:

On Mon, May 06, 2024 at 04:53:47PM +0200, Arnd Bergmann wrote:

On Mon, May 6, 2024, at 15:14, Daniel Vetter wrote:

On Fri, May 03, 2024 at 01:22:10PM -0700, Florian Fainelli wrote:

On 5/3/24 12:45, Arnd Bergmann wrote:


This is the current Android GKI config:
https://android.googlesource.com/kernel/common.git/+/refs/heads/android-mainline/arch/arm64/configs/gki_defconfig
where I can see that CONFIG_DRM is built-in, but DRM_FBDEV_EMULATION
CONFIG_VT, CONFIG_FRAMEBUFFER_CONSOLE, CONFIG_FB_DEVICE and
CONFIG_FB_CORE are all disabled.

So the console won't work at all,I think this means that there
is no way to get the console working, but building a fb.ko module
allows using /dev/fb with simplefb.ko (or any other one) happens
to almost work, but only by dumb luck rather than by design.


So using /dev/fb chardev without fbcon is very much a real idea. This way
you should be able to run old userspace that uses fbdev directly for
drawing, but your console needs are served by a userspace console running
on top of drm.

vt switching gets a bit more entertaining, but I thought logind has all
the glue already to make that happen. Worst case you need a tiny launcher
tool to get your userspace console out of the way while you launch a fbdev
using application, but I think correctly implement the vt ioctls to switch
to graphics mode /should/ work automatically.

I do agree that this is only really a good idea with drm drivers, since
those do not rely on any of the fbdev infrastructure like the notifier
under discussion.


I'm pretty sure what Florian is looking for has no dependency
on VT, fbcon or logind, but I'm only guessing based on the
information I see in the public Android source trees.

My understanding is that the Android recovery application is a
graphical tool that accesses the framebuffer directly and
is controlled using the volume and power buttons on a phone.


I suppose making CONFIG_FB_NOTIFIER optional for FB (on by
default if any of the consumers of the notification are turned
on) would not be a bad direction to go in general and also
address Florian's link error, but that doesn't solve the
more general concern about a third-party fb.ko module on a
kernel that was explicitly built with FB disabled.

The GKI defconfig was initially done at a time where one could
not have CONFIG_FBDEV_EMULATION and CONFIG_FB_DEVICE without
pulling in the entire fbdev module, but now that is possible.
Maybe that is something that Android could now include?

Alternatively, I wonder if that recovery image could be changed
to access the screen through the /dev/dri/ interfaces? I have
no experience in using those without Xorg or Wayland, is that
a sensible thing to do?


Uh ... I think I'm confused about the requirements. Does android's
recovery image need fbdev (meaning /dev/fb chardevs), or does it need
fbcon?

Note that fbcon runs (or well, should run) totally fine on top of drm
drivers without the fb notifier. This wasn't the case a few years ago
(because fbcon also used that notifier), but nowadays fb notifiers are
only needed for legacy fbdev drivers. So could it be that this "need fb
notifier" requirement is a leftover from rather old kernel versions and
not actually needed anymore?

I think we should nail the actual requirements here first before jumping
to solutions ...


Right, let's wait for Florian to reply. From what he said earlier
though, the only reason that the notifiers are getting in the
way is the link error you get from trying to load a separately
built fb.ko module on a kernel that was built with FB=n / FB_CORE=n,
so I don't think he even cares about notifiers, only about
allowing the recovery application to mmap() the framebuffer.


Right, we do not really care about notifiers AFAICT. Based upon this 
discussion there has been an action on our side to stop making use of 
the FB subsystem for recovery and use the full blow DRM driver instead.


While we get there, though I still see some value into this patch (or a 
v2, that is). I have a v2 ready if you think there is some value in 
pursuing that route, if not, we can stop there.

--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate

[Florian Fainelli]

On 5/3/24 12:45, Arnd Bergmann wrote:

On Fri, May 3, 2024, at 21:28, Florian Fainelli wrote:

Android devices in recovery mode make use of a framebuffer device to
provide an user interface. In a GKI configuration that has CONFIG_FB=m,
but CONFIG_FB_NOTIFY=y, loading the fb.ko module will fail with:

fb: Unknown symbol fb_notifier_call_chain (err -2)

Have CONFIG_FB_NOTIFY be tristate, just like CONFIG_FB such that both
can be loaded as module with fb_notify.ko first, and fb.ko second.

Signed-off-by: Florian Fainelli 


I see two problems here, so I don't think this is the right
approach:

  1. I don't understand your description: Since fb_notifier_call_chain()
 is an exported symbol, it should work for both FB_NOTIFY=y and
 FB_NOTIFY=m, unless something in Android drops the exported
 symbols for some reason.


The symbol is still exported in the Android tree. The issue is that the 
GKI defconfig does not enable any CONFIG_FB options at all. This is left 
for SoC vendors to do in their own "fragment" which would add 
CONFIG_FB=m. That implies CONFIG_FB_NOTIFY=y which was not part of the 
original kernel image we build/run against, and so we cannot resolve the 
symbol.


This could be resolved by having the GKI kernel have CONFIG_FB_NOTIFY=y 
but this is a bit wasteful (not by much since the code is very slim 
anyway) and it does require making changes specifically to the Android 
tree which could be beneficial upstream, hence my attempt at going 
upstream first.


IMHO it makes sense for all subsystem supporting code to be completely 
modular or completely built-in, or at least allowed to be.




  2. This breaks if any of the four callers of fb_register_client()
 are built-in while CONFIG_FB_NOTIFY is set to =m:


Ah good point, I missed that part, thanks, adding "select FB_NOTIFY" 
ought to be enough for those.




$ git grep -w fb_register_client
arch/arm/mach-pxa/am200epd.c:   fb_register_client(_fb_notif);
drivers/leds/trigger/ledtrig-backlight.c:   ret = 
fb_register_client(>notifier);
drivers/video/backlight/backlight.c:return 
fb_register_client(>fb_notif);
drivers/video/backlight/lcd.c:  return fb_register_client(>fb_notif);

Somewhat related but not directly addressing your patch, I wonder
if Android itself could migrate to using FB_CORE=m FB=n FB_NOTIFY=n
instead and use simpledrm for the console in place of the legacy
fbdev layer.


That is beyond my reach :)
--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate

[Florian Fainelli]

Android devices in recovery mode make use of a framebuffer device to
provide an user interface. In a GKI configuration that has CONFIG_FB=m,
but CONFIG_FB_NOTIFY=y, loading the fb.ko module will fail with:

fb: Unknown symbol fb_notifier_call_chain (err -2)

Have CONFIG_FB_NOTIFY be tristate, just like CONFIG_FB such that both
can be loaded as module with fb_notify.ko first, and fb.ko second.

Signed-off-by: Florian Fainelli 
---
 drivers/video/fbdev/core/Kconfig | 2 +-
 drivers/video/fbdev/core/fb_notify.c | 3 +++
 include/linux/fb.h   | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig
index db09fe87fcd4..036af8b5914a 100644
--- a/drivers/video/fbdev/core/Kconfig
+++ b/drivers/video/fbdev/core/Kconfig
@@ -8,7 +8,7 @@ config FB_CORE
tristate
 
 config FB_NOTIFY
-   bool
+   tristate
 
 config FIRMWARE_EDID
bool "Enable firmware EDID"
diff --git a/drivers/video/fbdev/core/fb_notify.c 
b/drivers/video/fbdev/core/fb_notify.c
index 10e3b9a74adc..ef707e092344 100644
--- a/drivers/video/fbdev/core/fb_notify.c
+++ b/drivers/video/fbdev/core/fb_notify.c
@@ -52,3 +52,6 @@ int fb_notifier_call_chain(unsigned long val, void *v)
return blocking_notifier_call_chain(_notifier_list, val, v);
 }
 EXPORT_SYMBOL_GPL(fb_notifier_call_chain);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Frame buffer notifier support");
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 0dd27364d56f..8c7ae5997278 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -156,7 +156,7 @@ struct fb_blit_caps {
u32 flags;
 };
 
-#ifdef CONFIG_FB_NOTIFY
+#if IS_ENABLED(CONFIG_FB_NOTIFY)
 extern int fb_register_client(struct notifier_block *nb);
 extern int fb_unregister_client(struct notifier_block *nb);
 extern int fb_notifier_call_chain(unsigned long val, void *v);
-- 
2.34.1

Re: [PATCH v2] ARM: dts: bcm2835: Enable 3D rendering through V3D

[Florian Fainelli]

On 4/16/24 10:21, Conor Dooley wrote:

On Tue, Apr 16, 2024 at 07:13:51AM -0300, Maíra Canal wrote:

On 4/16/24 02:30, Stefan Wahren wrote:

Hi Maíra,

Am 16.04.24 um 03:02 schrieb Maíra Canal:

On 4/15/24 13:54, Andre Przywara wrote:

On Mon, 15 Apr 2024 13:00:39 -0300
Maíra Canal  wrote:

Hi,


RPi 0-3 is packed with a GPU that provides 3D rendering capabilities to
the RPi. Currently, the downstream kernel uses an overlay to enable the
GPU and use GPU hardware acceleration. When deploying a mainline kernel
to the RPi 0-3, we end up without any GPU hardware acceleration
(essentially, we can't use the OpenGL driver).

Therefore, enable the V3D core for the RPi 0-3 in the mainline kernel.


So I think Krzysztof's initial comment still stands: What does that
patch
actually change? If I build those DTBs as of now, none of them has a
status property in the v3d node. Which means it's enabled:
https://github.com/devicetree-org/devicetree-specification/blob/main/source/chapter2-devicetree-basics.rst#status

So adding an explicit 'status = "okay";' doesn't make a difference.

What do I miss here?


As mentioned by Stefan in the last version, in Raspberry Pi OS, there is
a systemd script which is trying to check for the V3D driver (/usr/lib
/systemd/scripts/gldriver_test.sh). Within the first check, "raspi-
config nonint is_kms" is called, which always seems to fail. What
"raspi-config" does is check if
/proc/device-tree/soc/v3d@7ec0/status is equal to "okay". As
/proc/device-tree/soc/v3d@7ec0/status doesn't exists, it returns
false.

yes, but i also mention that the V3D driver starts without this patch.
The commit message of this patch suggests this is a DT issue, which is not.

I hadn't the time to update my SD card to Bookworm yet. Does the issue
still exists with this version?


I'm using a 32-bit kernel and the recommended OS for 32-bit is Bullseye.
But I checked the Bookworm code and indeed, Bookworm doesn't check
the device tree [1].

I'm thinking about sending a patch to the Bullseye branch to fix this
issue.


I think you should, sounds like they're making invalid assumptions about
the status property.


Agreed, fix the script, not the DTSes.
--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH] phy: constify of_phandle_args in xlate

[Florian Fainelli]

On 2/17/24 01:39, Krzysztof Kozlowski wrote:

The xlate callbacks are supposed to translate of_phandle_args to proper
provider without modifying the of_phandle_args.  Make the argument
pointer to const for code safety and readability.

Signed-off-by: Krzysztof Kozlowski 


Acked-by: Florian Fainelli  #Broadcom
--
Florian

Re: [PATCH v5 003/111] pwm: Provide a macro to get the parent device of a given chip

[Florian Fainelli]

On 1/25/24 04:08, Uwe Kleine-König wrote:

Currently a pwm_chip stores in its struct device *dev member a pointer
to the parent device. Preparing a change that embeds a full struct
device in struct pwm_chip, this accessor macro should be used in all
drivers directly accessing chip->dev now. This way struct pwm_chip and
this macro can be changed without having to touch all drivers in the
same change set.

Signed-off-by: Uwe Kleine-König 


Nit: this is not a macro but an inline function.
--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH] MAINTAINERS: Change vmware.com addresses to broadcom.com

[Florian Fainelli]

On 12/24/2023 6:20 AM, Zack Rusin wrote:

Update the email addresses for vmwgfx and vmmouse to reflect the fact
that VMware is now part of Broadcom.

Add a .mailmap entry because the vmware.com address will start bouncing
soon.

Signed-off-by: Zack Rusin 
Cc: Andrew Morton 
Cc: Ian Forbes 
Cc: Martin Krastev 
Cc: Maaz Mombasawala 
Cc: Broadcom internal kernel review list 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org


Acked-by: Florian Fainelli 
--
Florian


smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2 1/2] pwm: Manage owner assignment implicitly for drivers

[Florian Fainelli]

On 8/4/23 07:27, Uwe Kleine-König wrote:

Instead of requiring each driver to care for assigning the owner member
of struct pwm_ops, handle that implicitly using a macro. Note that the
owner member has to be moved to struct pwm_chip, as the ops structure
usually lives in read-only memory and so cannot be modified.

The upside is that new lowlevel drivers cannot forget the assignment and
save one line each. The pwm-crc driver didn't assign .owner, that's not
a problem in practise though as the driver cannot be compiled as a
module.

Signed-off-by: Uwe Kleine-König  > ---



  drivers/pwm/pwm-bcm-iproc.c   |  1 -
  drivers/pwm/pwm-bcm-kona.c|  1 -
  drivers/pwm/pwm-bcm2835.c |  1 -
  drivers/pwm/pwm-brcmstb.c |  1 -


Reviewed-by: Florian Fainelli  # 
pwm-{bcm,brcm}*.c

--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH net-next v6 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller

[Florian Fainelli]

On 6/2/2023 11:58 PM, Jakub Kicinski wrote:

On Thu,  1 Jun 2023 15:12:28 -0700 Justin Chen wrote:

+   /* general stats */
+   STAT_NETDEV(rx_packets),
+   STAT_NETDEV(tx_packets),
+   STAT_NETDEV(rx_bytes),
+   STAT_NETDEV(tx_bytes),
+   STAT_NETDEV(rx_errors),
+   STAT_NETDEV(tx_errors),
+   STAT_NETDEV(rx_dropped),
+   STAT_NETDEV(tx_dropped),
+   STAT_NETDEV(multicast),


please don't report standard interface stats in ethtool -S


+   /* UniMAC RSV counters */
+   STAT_BCMASP_MIB_RX("rx_64_octets", mib.rx.pkt_cnt.cnt_64),
+   STAT_BCMASP_MIB_RX("rx_65_127_oct", mib.rx.pkt_cnt.cnt_127),
+   STAT_BCMASP_MIB_RX("rx_128_255_oct", mib.rx.pkt_cnt.cnt_255),
+   STAT_BCMASP_MIB_RX("rx_256_511_oct", mib.rx.pkt_cnt.cnt_511),
+   STAT_BCMASP_MIB_RX("rx_512_1023_oct", mib.rx.pkt_cnt.cnt_1023),
+   STAT_BCMASP_MIB_RX("rx_1024_1518_oct", mib.rx.pkt_cnt.cnt_1518),
+   STAT_BCMASP_MIB_RX("rx_vlan_1519_1522_oct", mib.rx.pkt_cnt.cnt_mgv),
+   STAT_BCMASP_MIB_RX("rx_1522_2047_oct", mib.rx.pkt_cnt.cnt_2047),
+   STAT_BCMASP_MIB_RX("rx_2048_4095_oct", mib.rx.pkt_cnt.cnt_4095),
+   STAT_BCMASP_MIB_RX("rx_4096_9216_oct", mib.rx.pkt_cnt.cnt_9216),


these should also be removed, and you should implement @get_rmon_stats.


+   STAT_BCMASP_MIB_RX("rx_pkts", mib.rx.pkt),
+   STAT_BCMASP_MIB_RX("rx_bytes", mib.rx.bytes),
+   STAT_BCMASP_MIB_RX("rx_multicast", mib.rx.mca),
+   STAT_BCMASP_MIB_RX("rx_broadcast", mib.rx.bca),
+   STAT_BCMASP_MIB_RX("rx_fcs", mib.rx.fcs),


there's a FCS error statistic in the standard stats, no need to
duplicate


+   STAT_BCMASP_MIB_RX("rx_control", mib.rx.cf),
+   STAT_BCMASP_MIB_RX("rx_pause", mib.rx.pf),


@get_pause_stats


+   STAT_BCMASP_MIB_RX("rx_unknown", mib.rx.uo),
+   STAT_BCMASP_MIB_RX("rx_align", mib.rx.aln),
+   STAT_BCMASP_MIB_RX("rx_outrange", mib.rx.flr),
+   STAT_BCMASP_MIB_RX("rx_code", mib.rx.cde),
+   STAT_BCMASP_MIB_RX("rx_carrier", mib.rx.fcr),
+   STAT_BCMASP_MIB_RX("rx_oversize", mib.rx.ovr),
+   STAT_BCMASP_MIB_RX("rx_jabber", mib.rx.jbr),


these look like candidates from standard stats, too.
Please read thru:

https://docs.kernel.org/next/networking/statistics.html


+   STAT_BCMASP_MIB_RX("rx_mtu_err", mib.rx.mtue),
+   STAT_BCMASP_MIB_RX("rx_good_pkts", mib.rx.pok),
+   STAT_BCMASP_MIB_RX("rx_unicast", mib.rx.uc),
+   STAT_BCMASP_MIB_RX("rx_ppp", mib.rx.ppp),
+   STAT_BCMASP_MIB_RX("rx_crc", mib.rx.rcrc),


hm, what's the difference between rx_crc and rx_fcs ?


Since you are going to respin to address Jakub's feedback, we should 
also consider using the shared unimac.h header file. We could even make 
a library out of it for standardized statistics once this driver gets 
accepted.

--
Florian


smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH net-next v5 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller

[Florian Fainelli]

On 5/24/23 16:01, Justin Chen wrote:

Add support for the Broadcom ASP 2.0 Ethernet controller which is first
introduced with 72165. This controller features two distinct Ethernet
ports that can be independently operated.

This patch supports:

- Wake-on-LAN using magic packets
- basic ethtool operations (link, counters, message level)
- MAC destination address filtering (promiscuous, ALL_MULTI, etc.)

Reviewed-by: Simon Horman 
Signed-off-by: Florian Fainelli 
Signed-off-by: Justin Chen 
---


[snip]


+static const struct net_device_ops bcmasp_netdev_ops = {
+   .ndo_open   = bcmasp_open,
+   .ndo_stop   = bcmasp_stop,
+   .ndo_start_xmit = bcmasp_xmit,
+   .ndo_tx_timeout = bcmasp_tx_timeout,
+   .ndo_set_rx_mode= bcmasp_set_rx_mode,
+   .ndo_get_phys_port_name = bcmasp_get_phys_port_name,
+   .ndo_get_stats  = bcmasp_get_stats,
+   .ndo_do_ioctl   = bcmasp_ioctl,


This needs to be:

@@ -1207,7 +1196,7 @@ static const struct net_device_ops 
bcmasp_netdev_ops = {

.ndo_set_rx_mode= bcmasp_set_rx_mode,
.ndo_get_phys_port_name = bcmasp_get_phys_port_name,
.ndo_get_stats  = bcmasp_get_stats,
-   .ndo_do_ioctl   = bcmasp_ioctl,
+   .ndo_eth_ioctl  = phy_do_ioctl_running,
.ndo_set_mac_address= bcmasp_set_mac_address,
 };

such that MII ioctls work properly.
--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH net-next v5 2/6] dt-bindings: net: Brcm ASP 2.0 Ethernet controller

[Florian Fainelli]

On 5/31/23 12:31, Krzysztof Kozlowski wrote:

On 31/05/2023 21:29, Florian Fainelli wrote:

+required:
+  - reg
+  - brcm,channel
+
+additionalProperties: false
+
+patternProperties:
+  "^mdio@[0-9a-f]+$":


Isn't mdio a property of each ethernet port? Existing users
(e.g.bcmgenet, owl-emac, switches) do it that way...


They are sub-nodes of the larger Ethernet controller block, hence the
property here.


This is the Ethernet controller. They are subnodes here, so what do you
mean by that? They are part of some other block?


The block is not just an Ethernet controller it has other functions, 
which is why we went with a top-level node with a 'ranges' property. One 
of those functions are the MDIO bus controllers. The examples makes it 
reasonably clear.








Otherwise how do you define relation-ship? Can one mdio fit multiple ports?


The relationship is established between Ethernet ports and children
nodes of the MDIO controller, such as switches or Ethernet PHYs using
'phy-handle' for instance. And yes, a single/common MDIO controller
could be serving multiple Ethernet ports.


We do not talk about generic case, but your device.


The generic case is true here as well. We so happen to have a 1:1 
mapping between the MDIO controller, PHY, and Ethernet port, in this 
particular example.

--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH net-next v5 2/6] dt-bindings: net: Brcm ASP 2.0 Ethernet controller

[Florian Fainelli]

On 5/31/23 12:18, Krzysztof Kozlowski wrote:

On 25/05/2023 01:01, Justin Chen wrote:

From: Florian Fainelli 

Add a binding document for the Broadcom ASP 2.0 Ethernet
controller.

Signed-off-by: Florian Fainelli 
Signed-off-by: Justin Chen 
---
v5
- Fix compatible string yaml format to properly capture what we want

v4
 - Adjust compatible string example to reference SoC and HW ver

v3
 - Minor formatting issues
 - Change channel prop to brcm,channel for vendor specific format
 - Removed redundant v2.0 from compat string
 - Fix ranges field

v2
 - Minor formatting issues

  .../devicetree/bindings/net/brcm,asp-v2.0.yaml | 149 +
  1 file changed, 149 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml

diff --git a/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml 
b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml
new file mode 100644
index ..c4cd24492bfd
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml
@@ -0,0 +1,149 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/brcm,asp-v2.0.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom ASP 2.0 Ethernet controller
+
+maintainers:
+  - Justin Chen 
+  - Florian Fainelli 
+
+description: Broadcom Ethernet controller first introduced with 72165
+
+properties:
+  '#address-cells':


Judging by more comments, there will be a v6, thus please also use
consistent quotes - either ' or ".


+const: 1
+  '#size-cells':
+const: 1
+
+  compatible:


As Conor pointed out, compatible is always first.


+oneOf:
+  - items:
+  - enum:
+  - brcm,bcm74165-asp
+  - const: brcm,asp-v2.1
+  - items:
+  - enum:
+  - brcm,bcm72165-asp
+  - const: brcm,asp-v2.0
+
+  reg:
+maxItems: 1
+
+  ranges: true
+
+  interrupts:
+minItems: 1
+items:
+  - description: RX/TX interrupt
+  - description: Port 0 Wake-on-LAN
+  - description: Port 1 Wake-on-LAN
+
+  clocks:
+maxItems: 1
+
+  ethernet-ports:
+type: object
+properties:
+  '#address-cells':
+const: 1
+  '#size-cells':
+const: 0
+
+patternProperties:
+  "^port@[0-9]+$":
+type: object
+
+$ref: ethernet-controller.yaml#
+
+properties:
+  reg:
+maxItems: 1
+description: Port number
+
+  brcm,channel:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: ASP channel number


Why do you need it? reg defines it. Your description does not explain
here much, except copying property name. Can we please avoid
descriptions which just copy name?


+
+required:
+  - reg
+  - brcm,channel
+
+additionalProperties: false
+
+patternProperties:
+  "^mdio@[0-9a-f]+$":


Isn't mdio a property of each ethernet port? Existing users
(e.g.bcmgenet, owl-emac, switches) do it that way...


They are sub-nodes of the larger Ethernet controller block, hence the 
property here.




Otherwise how do you define relation-ship? Can one mdio fit multiple ports?


The relationship is established between Ethernet ports and children 
nodes of the MDIO controller, such as switches or Ethernet PHYs using 
'phy-handle' for instance. And yes, a single/common MDIO controller 
could be serving multiple Ethernet ports.

--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH net-next v3 1/6] dt-bindings: net: brcm,unimac-mdio: Add asp-v2.0

[Florian Fainelli]

On 5/22/23 11:17, Conor Dooley wrote:

On Fri, May 19, 2023 at 02:19:39PM -0700, Justin Chen wrote:
 > The ASP 2.0 Ethernet controller uses a brcm unimac.
 >
 > Signed-off-by: Florian Fainelli 
 > Signed-off-by: Justin Chen 
 > ---
 >  Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml | 2 ++
 >  1 file changed, 2 insertions(+)
 >
 > diff --git 
a/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml 
b/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml

 > index 0be426ee1e44..6684810fcbf0 100644
 > --- a/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml
 > +++ b/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml
 > @@ -22,6 +22,8 @@ properties:
 >    - brcm,genet-mdio-v3
 >    - brcm,genet-mdio-v4
 >    - brcm,genet-mdio-v5
 > +  - brcm,asp-v2.0-mdio
 > +  - brcm,asp-v2.1-mdio
 >    - brcm,unimac-mdio


 From V(N-1), there was some discussion between Rob & Florian:
 > > How many SoCs does each of these correspond to? SoC specific 
compatibles

 > > are preferred to version numbers (because few vendors are disciplined
 > > at versioning and also not changing versions with every Soc).
 >
 > So far there is a 1:1 mapping between the number of versions and the
 > number of SoCs, and the older SoC uses v2.0, while the newer one uses 
v2.1.


Rob's not around right now, but I don't really get why if there is a 1:1
mapping you don't just name these things after the SoCs?


There is a 1:1 mapping now, but in the future there may be more SoCs 
with a given implemented version. This is especially true for the MDIO 
controller which has been largely unchanged since it was introduced.




Also, my mailer **refused** to let me reply to you because of something
to do with a garbage S/MIME signature? Dunno wtf is happening there.


Our SMTP server is configured to automatically wrap the message in a 
S/MIME envelope, nothing invalid though AFAICT. What's your email client?

--
Florian



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2 net-next 1/6] dt-bindings: net: brcm,unimac-mdio: Add asp-v2.0

[Florian Fainelli]

On 4/27/23 10:03, Rob Herring wrote:

On Wed, Apr 26, 2023 at 11:54:27AM -0700, Justin Chen wrote:

The ASP 2.0 Ethernet controller uses a brcm unimac.

Signed-off-by: Florian Fainelli 
Signed-off-by: Justin Chen 
---
  Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml 
b/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml
index 0be426ee1e44..6684810fcbf0 100644
--- a/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml
+++ b/Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml
@@ -22,6 +22,8 @@ properties:
- brcm,genet-mdio-v3
- brcm,genet-mdio-v4
- brcm,genet-mdio-v5
+  - brcm,asp-v2.0-mdio
+  - brcm,asp-v2.1-mdio


How many SoCs does each of these correspond to? SoC specific compatibles
are preferred to version numbers (because few vendors are disciplined
at versioning and also not changing versions with every Soc).


So far there is a 1:1 mapping between the number of versions and the 
number of SoCs, and the older SoC uses v2.0, while the newer one uses v2.1.

--
Florian

Re: [PATCH net-next 1/6] dt-bindings: net: Brcm ASP 2.0 Ethernet controller

[Florian Fainelli]

On 4/24/23 11:14, Justin Chen wrote:

On Fri, Apr 21, 2023 at 12:29 AM Krzysztof Kozlowski
 wrote:


On 19/04/2023 02:10, Justin Chen wrote:

From: Florian Fainelli 

Add a binding document for the Broadcom ASP 2.0 Ethernet
controller.

Signed-off-by: Florian Fainelli 
Signed-off-by: Justin Chen 
---
  .../devicetree/bindings/net/brcm,asp-v2.0.yaml | 146 +
  1 file changed, 146 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml

diff --git a/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml 
b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml
new file mode 100644
index ..3817d722244f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml
@@ -0,0 +1,146 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/brcm,asp-v2.0.yaml#;
+$schema: "http://devicetree.org/meta-schemas/core.yaml#;


Drop quotes.


+
+title: Broadcom ASP 2.0 Ethernet controller
+
+maintainers:
+  - Justin Chen 
+  - Florian Fainelli 
+
+description: Broadcom Ethernet controller first introduced with 72165
+
+properties:
+  '#address-cells':
+const: 1
+  '#size-cells':
+const: 1
+
+  compatible:
+enum:
+  - brcm,bcm72165-asp-v2.0
+  - brcm,asp-v2.0
+  - brcm,asp-v2.1


Is this part of SoC? If so, then SoC compatibles are preferred, not IP
block versions.

We have the same IP on different chips. So no, it isn't tied to a specific SoC.




+
+  reg:
+maxItems: 1
+description: ASP registers


Drop description.


+
+  ranges: true
+
+  interrupts:
+minItems: 1
+items:
+  - description: RX/TX interrupt
+  - description: Port 0 Wake-on-LAN
+  - description: Port 1 Wake-on-LAN
+
+  clocks:
+$ref: /schemas/types.yaml#/definitions/phandle-array


Drop.


+description: Phandle to clock controller


Drop.

Instead maxItems.


+
+  clock-names:
+const: sw_asp


Drop entire property.


+
+  ethernet-ports:
+type: object
+properties:
+  '#address-cells':
+const: 1
+  '#size-cells':
+const: 0


Missing additionalProperties:false. Look at existing bindings how it is
done.


+
+patternProperties:
+  "^port@[0-9]+$":
+type: object
+
+$ref: ethernet-controller.yaml#
+
+properties:
+  reg:
+maxItems: 1
+description: Port number
+
+  channel:
+maxItems: 1
+description: ASP channel number
+
+required:
+  - reg
+  - channel
+
+patternProperties:
+  "^mdio@[0-9a-f]+$":
+type: object
+$ref: "brcm,unimac-mdio.yaml"
+
+description:
+  ASP internal UniMAC MDIO bus
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+asp@9c0 {


Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


+compatible = "brcm,asp-v2.0";
+reg = <0x9c0 0x1fff14>;
+interrupts = <0x0 0x33 0x4>;


Use proper defines for flags.

Not understanding this comment. Can you elaborate?


I believe Krzysztof would prefer that you use:

interrupts = 

here, which would require using defined from 
include/dt-bindings/interrupt-controller/{arm-gic.h,irq.h}

--
Florian

Re: [PATCH net-next 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller

[Florian Fainelli]

On 4/18/23 23:35, Heiner Kallweit wrote:

On 19.04.2023 02:10, Justin Chen wrote:

Add support for the Broadcom ASP 2.0 Ethernet controller which is first
introduced with 72165. This controller features two distinct Ethernet
ports that can be independently operated.

This patch supports:

[snip]

+   intf->tx_spb_index = spb_index;
+   intf->tx_spb_dma_valid = valid;
+   bcmasp_intf_tx_write(intf, intf->tx_spb_dma_valid);
+
+   if (tx_spb_ring_full(intf, MAX_SKB_FRAGS + 1))
+   netif_stop_queue(dev);
+


Here it may be better to use the new macros from include/net/netdev_queues.h.
It seems your code (together with the related part in tx_poll) doesn't consider
the queue restart case.
In addition you should check whether using READ_ONCE()/WRITE_ONCE() is needed,
e.g. in ring_full().


Thanks Heiner. Can you trim the parts you are not quoting otherwise one 
has to scroll all the way down to where you responded. Thanks!

--
Florian

Re: [PATCH v5 0/7] drm/vc4: Fix the core clock behaviour

[Florian Fainelli]

On 10/27/2022 7:59 AM, Maxime Ripard wrote:

Hi Florian,

On Thu, Oct 27, 2022 at 02:52:40PM +0200, Maxime Ripard wrote:

Hi,

Those patches used to be part of a larger clock fixes series:
https://lore.kernel.org/linux-clk/20220715160014.2623107-1-max...@cerno.tech/

However, that series doesn't seem to be getting anywhere, so I've split out
these patches that fix a regression that has been there since 5.18 and that
prevents the 4k output from working on the RaspberryPi4.

Hopefully, we will be able to merge those patches through the DRM tree to avoid
any further disruption.


I intend to get this through drm-misc, but you just gave your
Reviewed-by on all the firmware patches but the first. Are you ok with
this? If so, can I add your Acked-by?


I don't feel legitimate on any of those patches, but the firmware part 
is something that I can understand, gave you an Acked-by for patch #3, 
hopefully this allows you to merge through drm-misc now.

--
Florian

Re: [PATCH v5 3/7] firmware: raspberrypi: Provide a helper to query a clock max rate

[Florian Fainelli]

On 10/27/2022 5:52 AM, Maxime Ripard wrote:

The firmware allows to query for its clocks the operating range of a
given clock. We'll need this for some drivers (KMS, in particular) to
infer the state of some configuration options, so let's create a
function to do so.

Acked-by: Stephen Boyd 
Reviewed-by: Florian Fainelli 
Signed-off-by: Maxime Ripard 


Acked-by: Florian Fainelli 
--
Florian

Re: [PATCH v4 1/7] firmware: raspberrypi: Introduce rpi_firmware_find_node()

[Florian Fainelli]

On 10/20/22 02:12, max...@cerno.tech wrote:

A significant number of RaspberryPi drivers using the firmware don't
have a phandle to it, so end up scanning the device tree to find a node
with the firmware compatible.

That code is duplicated everywhere, so let's introduce a helper instead.

Signed-off-by: Maxime Ripard 


Acked-by: Florian Fainelli 

Thanks for re-ordering the rpi_firmware_of_match array and avoiding a 
forward declaration that I was initially confused about in v3.

--
Florian

Re: [PATCH v3 3/7] firmware: raspberrypi: Provide a helper to query a clock max rate

[Florian Fainelli]

On 10/13/22 02:13, Maxime Ripard wrote:

The firmware allows to query for its clocks the operating range of a
given clock. We'll need this for some drivers (KMS, in particular) to
infer the state of some configuration options, so let's create a
function to do so.

Acked-by: Stephen Boyd 
Signed-off-by: Maxime Ripard 


Reviewed-by: Florian Fainelli 
--
Florian

Re: [PATCH v3 2/7] firmware: raspberrypi: Move the clock IDs to the firmware header

[Florian Fainelli]

On 10/13/22 02:13, Maxime Ripard wrote:

We'll need the clock IDs in more drivers than just the clock driver from
now on, so let's move them in the firmware header.

Signed-off-by: Maxime Ripard 


Reviewed-by: Florian Fainelli 
--
Florian

Re: [PATCH v3 1/7] firmware: raspberrypi: Introduce rpi_firmware_find_node()

[Florian Fainelli]

On 10/13/22 02:13, Maxime Ripard wrote:

A significant number of RaspberryPi drivers using the firmware don't
have a phandle to it, so end up scanning the device tree to find a node
with the firmware compatible.

That code is duplicated everywhere, so let's introduce a helper instead.

Signed-off-by: Maxime Ripard 
---
  drivers/firmware/raspberrypi.c | 7 +++
  include/soc/bcm2835/raspberrypi-firmware.h | 7 +++
  2 files changed, 14 insertions(+)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 4b8978b254f9..b916e1e171f8 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -311,6 +311,13 @@ static int rpi_firmware_remove(struct platform_device 
*pdev)
return 0;
  }
  
+static const struct of_device_id rpi_firmware_of_match[];


This shadows the same variable that is used later for matching the 
firmware driver and probe it as a platform_driver, what am I missing here?

--
Florian

Re: [PATCH v2 0/7] drm/vc4: Fix the core clock behaviour

[Florian Fainelli]

On 10/10/22 04:44, Maxime Ripard wrote:

Hi Florian,

On Tue, Sep 20, 2022 at 02:50:19PM +0200, Maxime Ripard wrote:

Those patches used to be part of a larger clock fixes series:
https://lore.kernel.org/linux-clk/20220715160014.2623107-1-max...@cerno.tech/

However, that series doesn't seem to be getting anywhere, so I've split out
these patches that fix a regression that has been there since 5.18 and that
prevents the 4k output from working on the RaspberryPi4.

Hopefully, we will be able to merge those patches through the DRM tree to avoid
any further disruption.


Could you review this? Ideally this would be merged through drm-misc due
to the dependencies between the new firmware functions and the DRM
patches.


I suppose I can review the firmware parts if you would like me to, for 
vc4 I am pretty much clueless, and despite efforts from Emma to get the 
vc4 driver to be usable on platforms other than Pi, that never happened 
unfortunately. It would be better to keep the firmware and vc4 drivers 
decoupled, just so "wrong" assumptions are not made, but for all 
practical purposes this is the only combination that exists.

--
Florian

Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume

[Florian Fainelli]

On 8/15/22 07:12, Maxime Ripard wrote:

On Wed, Aug 10, 2022 at 10:33:48PM +0200, Stefan Wahren wrote:

Hi Florian,

Am 09.08.22 um 21:02 schrieb Florian Fainelli:

On 8/4/22 16:11, Florian Fainelli wrote:

On 6/13/22 07:47, Maxime Ripard wrote:

From: Dave Stevenson 

The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain
attached to the HDMI block, handled in Linux through runtime_pm.

That power domain is shared with the VEC block, so even if we put our
runtime_pm reference in the HDMI driver it would keep being on. If the
VEC is disabled though, the power domain would be disabled and we would
lose any initialization done in our bind implementation.

That initialization involves calling the reset function and
initializing
the CEC registers.

Let's move the initialization to our runtime_resume implementation so
that we initialize everything properly if we ever need to.

Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable
to runtime_pm")
Signed-off-by: Dave Stevenson 
Signed-off-by: Maxime Ripard 


After seeing the same warning as Stefan reported in the link below,
but on the Raspberry Pi 4B:

https://www.spinics.net/lists/dri-devel/msg354170.html

a separate bisection effort led me to this commit, before is fine,
after produces 4 warnings during boot, see attached log.

Is there a fix that we can try that would also cover the Raspberry
Pi 4B? Is it possible that this series precipitates the problem:

https://www.spinics.net/lists/arm-kernel/msg984638.html


Maxime, Dave, anything you would want me to try? Still seeing these
warnings with net-next-6.0-11220-g15205c2829ca


At first this issue doesn't occur in Linux 5.19. So it's something new. I
was able to reproduce with todays linux-next, but interestingly it doesn't
occur in drm-misc-next.


Yeah, it should be fixed by
https://lore.kernel.org/all/20220629123510.1915022-38-max...@cerno.tech/
https://lore.kernel.org/all/20220629123510.1915022-39-max...@cerno.tech/

Both patches apparently didn't make the cut for the merge window, if it
works for you we can always queue them in drm-misc-fixes


Both of these patches eliminate the warning, I don't have a good set-up 
yet for ensuring that HDMI/V3dD is functional however:


Tested-by: Florian Fainelli 
--
Florian

Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume

[Florian Fainelli]

Hallo Stefan,

On 8/9/22 13:16, Stefan Wahren wrote:

Hi Florian,

Am 05.08.22 um 01:11 schrieb Florian Fainelli:

On 6/13/22 07:47, Maxime Ripard wrote:

From: Dave Stevenson 

The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain
attached to the HDMI block, handled in Linux through runtime_pm.

That power domain is shared with the VEC block, so even if we put our
runtime_pm reference in the HDMI driver it would keep being on. If the
VEC is disabled though, the power domain would be disabled and we would
lose any initialization done in our bind implementation.

That initialization involves calling the reset function and initializing
the CEC registers.

Let's move the initialization to our runtime_resume implementation so
that we initialize everything properly if we ever need to.

Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to 
runtime_pm")

Signed-off-by: Dave Stevenson 
Signed-off-by: Maxime Ripard 


After seeing the same warning as Stefan reported in the link below, 
but on the Raspberry Pi 4B:


https://www.spinics.net/lists/dri-devel/msg354170.html

a separate bisection effort led me to this commit, before is fine, 
after produces 4 warnings during boot, see attached log.

Which config do you use (multi_v7_defconfig + LPAE or arm64/defconfig)?


This was actually bcm2835_defconfig copied over to arch/arm64/configs/ 
and slightly modified to enable PCIe, here is it:


https://gist.github.com/481999edc11b823d0c3e87ecf1693d26



Is there a fix that we can try that would also cover the Raspberry Pi 
4B? Is it possible that this series precipitates the problem:


https://www.spinics.net/lists/arm-kernel/msg984638.html


I don't think this is related because this is a different driver.

Best regards




--
Florian

Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume

[Florian Fainelli]

On 8/4/22 16:11, Florian Fainelli wrote:

On 6/13/22 07:47, Maxime Ripard wrote:

From: Dave Stevenson 

The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain
attached to the HDMI block, handled in Linux through runtime_pm.

That power domain is shared with the VEC block, so even if we put our
runtime_pm reference in the HDMI driver it would keep being on. If the
VEC is disabled though, the power domain would be disabled and we would
lose any initialization done in our bind implementation.

That initialization involves calling the reset function and initializing
the CEC registers.

Let's move the initialization to our runtime_resume implementation so
that we initialize everything properly if we ever need to.

Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to 
runtime_pm")

Signed-off-by: Dave Stevenson 
Signed-off-by: Maxime Ripard 


After seeing the same warning as Stefan reported in the link below, but 
on the Raspberry Pi 4B:


https://www.spinics.net/lists/dri-devel/msg354170.html

a separate bisection effort led me to this commit, before is fine, after 
produces 4 warnings during boot, see attached log.


Is there a fix that we can try that would also cover the Raspberry Pi 
4B? Is it possible that this series precipitates the problem:


https://www.spinics.net/lists/arm-kernel/msg984638.html


Maxime, Dave, anything you would want me to try? Still seeing these 
warnings with net-next-6.0-11220-g15205c2829ca


Would be nice to see those fixes before 6.0 final, thanks!
--
Florian

Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume

[Florian Fainelli]

On 6/13/22 07:47, Maxime Ripard wrote:

From: Dave Stevenson 

The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain
attached to the HDMI block, handled in Linux through runtime_pm.

That power domain is shared with the VEC block, so even if we put our
runtime_pm reference in the HDMI driver it would keep being on. If the
VEC is disabled though, the power domain would be disabled and we would
lose any initialization done in our bind implementation.

That initialization involves calling the reset function and initializing
the CEC registers.

Let's move the initialization to our runtime_resume implementation so
that we initialize everything properly if we ever need to.

Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm")
Signed-off-by: Dave Stevenson 
Signed-off-by: Maxime Ripard 


After seeing the same warning as Stefan reported in the link below, but 
on the Raspberry Pi 4B:


https://www.spinics.net/lists/dri-devel/msg354170.html

a separate bisection effort led me to this commit, before is fine, after 
produces 4 warnings during boot, see attached log.


Is there a fix that we can try that would also cover the Raspberry Pi 
4B? Is it possible that this series precipitates the problem:


https://www.spinics.net/lists/arm-kernel/msg984638.html
--
FlorianStarting start4.elf @ 0xfec00200 partition -1
[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 5.19.0-rc2 (florian@silverado) (arm-buildroot-linux-gnueabi-gcc.br_real (Buildroot 2022.05-448-g7ff22c698a0d) 11.3.0, GNU ld (GNU Binutils)
 2.37) #73 SMP Thu Aug 4 16:09:03 PDT 2022
[0.00] CPU: ARMv7 Processor [410fd083] revision 3 (ARMv7), cr=30c5383d
[0.00] CPU: div instructions available: patching division code
[0.00] CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
[0.00] OF: fdt: Machine model: Raspberry Pi 4 Model B Rev 1.1
[0.00] earlycon: bcm2835aux0 at MMIO32 0xfe215040 (options '115200n8')
[0.00] printk: bootconsole [bcm2835aux0] enabled
[0.00] Memory policy: Data cache writealloc
[0.00] Reserved memory: created CMA memory pool at 0x3740, size 64 MiB
[0.00] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool
[0.00] Zone ranges:
[0.00]   DMA  [mem 0x-0x2fff]
[0.00]   Normal   empty
[0.00]   HighMem  [mem 0x3000-0xfbff]
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x-0x3b3f]
[0.00]   node   0: [mem 0x4000-0xfbff]
[0.00] Initmem setup node 0 [mem 0x-0xfbff]
[0.00] [ cut here ]
[0.00] WARNING: CPU: 0 PID: 0 at arch/arm/mm/physaddr.c:40 __virt_to_phys+0x84/0xbc
[0.00] virt_to_phys used for non-linear address:  (0x0)
[0.00] Modules linked in:
[0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc2 #73
[0.00] Hardware name: BCM2711
[0.00]  unwind_backtrace from show_stack+0x18/0x1c
[0.00]  show_stack from dump_stack_lvl+0x40/0x4c
[0.00]  dump_stack_lvl from __warn+0xb0/0x12c
[0.00]  __warn from warn_slowpath_fmt+0x80/0xc0
[0.00]  warn_slowpath_fmt from __virt_to_phys+0x84/0xbc
[0.00]  __virt_to_phys from pcpu_embed_first_chunk+0x588/0x7cc
[0.00]  pcpu_embed_first_chunk from setup_per_cpu_areas+0x24/0xa0
[0.00]  setup_per_cpu_areas from start_kernel+0x1a8/0x6b8
[0.00]  start_kernel from 0x0
[0.00] ---[ end trace  ]---
[0.00] percpu: Embedded 16 pages/cpu s35860 r8192 d21484 u65536
[0.00] Built 1 zonelists, mobility grouping on.  Total pages: 1011200
[0.00] Kernel command line:  dma.dmachans=0x71f5 bcm2709.boardrev=0xc03111 bcm2709.serial=0x4b11cb83 bcm2709.uart_clock=4800 bcm2709.disk_led_gpio=42 bcm270
9.disk_led_active_low=0 smsc95xx.macaddr=DC:A6:32:1C:A0:82 vc_mem.mem_base=0x3ec0 vc_mem.mem_size=0x4000  earlycon earlyprintk
[0.00] Unknown kernel command line parameters "earlyprintk", will be passed to user space.
[0.00] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes, linear)
[0.00] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes, linear)
[0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
[0.00] software IO TLB: mapped [mem 0x29079000-0x2d079000] (64MB)
[0.00] Memory: 3851564K/4050944K available (10240K kernel code, 1703K rwdata, 3832K rodata, 16384K init, 478K bss, 133844K reserved, 65536K cma-reserved, 319897
6K highmem)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[0.00] ftrace: allocating 38049 entries in 112 pages
[0.00] ftrace: allocated 

Re: [PATCH v6 6/6] arm64: config: Enable DRM_V3D

[Florian Fainelli]

On Fri,  3 Jun 2022 10:26:10 +0100, Peter Robinson  wrote:
> From: Nicolas Saenz Julienne 
> 
> BCM2711, the SoC used on the Raspberry Pi 4 has a different GPU than its
> predecessors. Enable it.
> 
> Signed-off-by: Nicolas Saenz Julienne 
> Signed-off-by: Peter Robinson 
> Reviewed-by: Stefan Wahren 
> Reviewed-by: Javier Martinez Canillas 
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree-arm64/next, 
thanks!
--
Florian

Re: [PATCH v6 5/6] ARM: configs: Enable DRM_V3D

[Florian Fainelli]

On Fri,  3 Jun 2022 10:26:09 +0100, Peter Robinson  wrote:
> BCM2711, the SoC used on the Raspberry Pi 4 has a different 3D
> render GPU IP than its predecessors. Enable it it on multi v7
> and bcm2835 configs.
> 
> Signed-off-by: Nicolas Saenz Julienne 
> Signed-off-by: Peter Robinson 
> Reviewed-by: Stefan Wahren 
> Reviewed-by: Javier Martinez Canillas 
> ---

Applied to https://github.com/Broadcom/stblinux/commits/defconfig/next, thanks!
--
Florian

Re: [PATCH v6 4/6] ARM: dts: bcm2711: Enable V3D

[Florian Fainelli]

On Fri,  3 Jun 2022 10:26:08 +0100, Peter Robinson  wrote:
> This adds the entry for V3D for bcm2711 (used in the Raspberry Pi 4)
> and the associated firmware clock entry.
> 
> Signed-off-by: Nicolas Saenz Julienne 
> Signed-off-by: Peter Robinson 
> Reviewed-by: Javier Martinez Canillas 
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks!
--
Florian

Re: [PATCH v6 0/6] Raspberry PI 4 V3D enablement

[Florian Fainelli]

On 6/3/2022 11:26 AM, Peter Robinson wrote:

This is a follow up from my v4 patchset. The power management pieces have
been split out to a separate independent set of patches by Stefan [1]. This
version 5 of the DRM patches are independent and given the V3D driver has
been upstream for some time the two patches to enable it in defconfigs can
be taken at anytime independent of the enablement for the Raspberry Pi 4.

I've tested this using mesa 22.0.x and Wayland/Gnome on Fedora 36, it's
more or less stable with basic testing.

Changes since v5:
- Update the DT compatible to match the others that were updated
- Adjust the Kconfig help text
- Add review tags

Changes since v4:
- Fixes for device tree and bindings
- Split out the power management changes into an independent set
- Rebase to 5.18
- Individual changes in patches

[1] https://www.spinics.net/lists/arm-kernel/msg980342.html


I can take the last 3 patches through the Broadcom ARM SoC pull request, 
but the first three should probably go via the DRM tree unless you want 
me to merge them all?

--
Florian

Re: [PATCH net-next 3/5] net: bcmasp: Add support for ASP2.0 Ethernet controller

[Florian Fainelli]

On 9/25/2021 9:45 AM, Andrew Lunn wrote:
[snip]


+   priv->clk = devm_clk_get(dev, "sw_asp");
+   if (IS_ERR(priv->clk)) {
+   if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   dev_warn(dev, "failed to request clock\n");
+   priv->clk = NULL;
+   }


devm_clk_get_optional() makes this simpler/


Indeed, thanks.

[snip]


+   ret = devm_request_irq(>dev, priv->irq, bcmasp_isr, 0,
+  pdev->name, priv);
+   if (ret) {
+   dev_err(dev, "failed to request ASP interrupt: %d\n", ret);
+   return ret;
+   }


Do you need to undo clk_prepare_enable()?


Yes we do need to undo the preceding clk_prepare_enable(), thanks!

[snip]


+
+static int bcmasp_remove(struct platform_device *pdev)
+{
+   struct bcmasp_priv *priv = dev_get_drvdata(>dev);
+   struct bcmasp_intf *intf;
+   int i;
+
+   for (i = 0; i < priv->intf_count; i++) {
+   intf = priv->intfs[i];
+   if (!intf)
+   continue;
+
+   bcmasp_interface_destroy(intf, true);
+   }
+
+   return 0;
+}


Do you need to depopulate the mdio children?


I had not given much thought about it first but we ought to do something 
about it here, I will test it before Justin sends a v2.





+static void bcmasp_get_drvinfo(struct net_device *dev,
+  struct ethtool_drvinfo *info)
+{
+   strlcpy(info->driver, "bcmasp", sizeof(info->driver));
+   strlcpy(info->version, "v2.0", sizeof(info->version));


Please drop version. The core will fill it in with the kernel version,
which is more useful.


+static int bcmasp_nway_reset(struct net_device *dev)
+{
+   if (!dev->phydev)
+   return -ENODEV;
+
+   return genphy_restart_aneg(dev->phydev);
+}


phy_ethtool_nway_reset().


Yes, thanks!





+static void bcmasp_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+   struct bcmasp_intf *intf = netdev_priv(dev);
+
+   wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
+   wol->wolopts = intf->wolopts;
+   memset(wol->sopass, 0, sizeof(wol->sopass));
+
+   if (wol->wolopts & WAKE_MAGICSECURE)
+   memcpy(wol->sopass, intf->sopass, sizeof(intf->sopass));
+}


Maybe consider calling into the PHY to see what it can do? If the PHY
can do the WoL you want, it will do it with less power.


We could do that, especially since one of the ports will typically 
connect to an external Gigabit PHY, will add for v2.





+static int bcmasp_set_priv_flags(struct net_device *dev, u32 flags)
+{
+   struct bcmasp_intf *intf = netdev_priv(dev);
+
+   intf->wol_keep_rx_en = flags & BCMASP_WOL_KEEP_RX_EN ? 1 : 0;
+
+   return 0;


Please could you explain this some more. How can you disable RX and
still have WoL working?


Wake-on-LAN using Magic Packets and network filters requires keeping the 
UniMAC's receiver turned on, and then the packets feed into the Magic 
Packet Detector (MPD) block or the network filter block. In that mode 
DRAM is in self refresh and there is local matching of frames into a 
tiny FIFO however in the case of magic packets the packets leading to a 
wake-up are dropped as there is nowhere to store them. In the case of a 
network filter match (e.g.: matching a multicast IP address plus 
protocol, plus source/destination ports) the packets are also discarded 
because the receive DMA was shut down.


When the wol_keep_rx_en flag is set, the above happens but we also allow 
the packets that did match a network filter to reach the small FIFO 
(Justin would know how many entries are there) that is used to push the 
packets to DRAM. The packet contents are held in there until the system 
wakes up which is usually just a few hundreds of micro seconds after we 
received a packet that triggered a wake-up. Once we overflow the receive 
DMA FIFO capacity subsequent packets get dropped which is fine since we 
are usually talking about very low bit rates, and we only try to push to 
DRAM the packets of interest, that is those for which we have a network 
filter.


This is convenient in scenarios where you want to wake-up from multicast 
DNS (e.g.: wake on Googlecast, Bonjour etc.) because then the packet 
that resulted in the system wake-up is not discarded but is then 
delivered to the network stack.





+static void bcmasp_adj_link(struct net_device *dev)
+{
+   struct bcmasp_intf *intf = netdev_priv(dev);
+   struct phy_device *phydev = dev->phydev;
+   int changed = 0;
+   u32 cmd_bits = 0, reg;
+
+   if (intf->old_link != phydev->link) {
+   changed = 1;
+   intf->old_link = phydev->link;
+   }
+
+   if (intf->old_duplex != phydev->duplex) {
+   changed = 1;
+   intf->old_duplex = phydev->duplex;
+   }
+
+   switch (phydev->speed) {
+   case 

Re: [PATCH net-next 0/5] brcm ASP 2.0 Ethernet controller

[Florian Fainelli]

Hi Andrew,

On 9/25/2021 7:25 AM, Andrew Lunn wrote:

On Fri, Sep 24, 2021 at 02:44:46PM -0700, Justin Chen wrote:

This patch set adds support for Broadcom's ASP 2.0 Ethernet controller.



Hi Justin

Does the hardware support L2 switching between the two ports? I'm just
wondering if later this is going to be modified into a switchdev
driver?


It does not, these are just a bunch of Ethernet ports sharing a few 
resources (clocks, network filters etc.).

--
Florian

Re: [PATCH v2 12/12] ARM: dts: bcm2711: Tune DMA parameters for HDMI audio

[Florian Fainelli]

On 5/25/2021 6:23 AM, Maxime Ripard wrote:
> From: Dom Cobley 
> 
> Enable NO_WAIT_RESP, DMA_WIDE_SOURCE, DMA_WIDE_DEST, and bump the DMA
> panic and AXI priorities to avoid any DMA transfer error with HBR audio
> (8 channel, 192Hz).
> 
> Signed-off-by: Dom Cobley 
> Signed-off-by: Maxime Ripard 
> ---
>  arch/arm/boot/dts/bcm2711.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> index 720beec54d61..9d1dde973680 100644
> --- a/arch/arm/boot/dts/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/bcm2711.dtsi
> @@ -344,7 +344,7 @@ hdmi0: hdmi@7ef00700 {
>   interrupt-names = "cec-tx", "cec-rx", "cec-low",
> "wakeup", "hpd-connected", 
> "hpd-removed";
>   ddc = <>;
> - dmas = < 10>;
> + dmas = < (10 | (1 << 27) | (1 << 24)| (15 << 20) | 
> (10 << 16))>;

This uses DT as a configuration language rather than a description here,
but this is most certainly an established practice that the bcm283-dma.c
supports, with no validation of the various arguments.. great.

Is there at least an option to move the meaning of this bitfields into
include/dt-bindings/dma/bcm2835-dma.h or something like that?
-- 
Florian

Re: [PATCH v7 02/15] swiotlb: Refactor swiotlb_create_debugfs

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Split the debugfs creation to make the code reusable for supporting
> different bounce buffer pools, e.g. restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian

Re: [PATCH v7 07/15] swiotlb: Update is_swiotlb_active to add a struct device argument

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Update is_swiotlb_active to add a struct device argument. This will be
> useful later to allow for restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian

Re: [PATCH v7 06/15] swiotlb: Update is_swiotlb_buffer to add a struct device argument

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Update is_swiotlb_buffer to add a struct device argument. This will be
> useful later to allow for restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian

Re: [PATCH v7 05/15] swiotlb: Add a new get_io_tlb_mem getter

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new getter, get_io_tlb_mem, to help select the io_tlb_mem struct.
> The restricted DMA pool is preferred if available.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian

Re: [PATCH v7 03/15] swiotlb: Add DMA_RESTRICTED_POOL

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new kconfig symbol, DMA_RESTRICTED_POOL, for restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian

Re: [PATCH v7 11/15] dma-direct: Add a new wrapper __dma_direct_free_pages()

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new wrapper __dma_direct_free_pages() that will be useful later
> for swiotlb_free().
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian

Re: [PATCH v7 04/15] swiotlb: Add restricted DMA pool initialization

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add the initialization function to create restricted DMA pools from
> matching reserved-memory nodes.
> 
> Signed-off-by: Claire Chang 
> ---
>  include/linux/device.h  |  4 +++
>  include/linux/swiotlb.h |  3 +-
>  kernel/dma/swiotlb.c| 76 +
>  3 files changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 38a2071cf776..4987608ea4ff 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -416,6 +416,7 @@ struct dev_links_info {
>   * @dma_pools:   Dma pools (if dma'ble device).
>   * @dma_mem: Internal for coherent mem override.
>   * @cma_area:Contiguous memory area for dma allocations
> + * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override.
>   * @archdata:For arch-specific additions.
>   * @of_node: Associated device tree node.
>   * @fwnode:  Associated device node supplied by platform firmware.
> @@ -521,6 +522,9 @@ struct device {
>  #ifdef CONFIG_DMA_CMA
>   struct cma *cma_area;   /* contiguous memory area for dma
>  allocations */
> +#endif
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> + struct io_tlb_mem *dma_io_tlb_mem;
>  #endif
>   /* arch specific additions */
>   struct dev_archdata archdata;
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 216854a5e513..03ad6e3b4056 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -72,7 +72,8 @@ extern enum swiotlb_force swiotlb_force;
>   *   range check to see if the memory was in fact allocated by this
>   *   API.
>   * @nslabs:  The number of IO TLB blocks (in groups of 64) between @start and
> - *   @end. This is command line adjustable via setup_io_tlb_npages.
> + *   @end. For default swiotlb, this is command line adjustable via
> + *   setup_io_tlb_npages.
>   * @used:The number of used IO TLB block.
>   * @list:The free list describing the number of free entries available
>   *   from each index.
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index b849b01a446f..1d8eb4de0d01 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -39,6 +39,13 @@
>  #ifdef CONFIG_DEBUG_FS
>  #include 
>  #endif
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#endif
>  
>  #include 
>  #include 
> @@ -690,3 +697,72 @@ static int __init swiotlb_create_default_debugfs(void)
>  late_initcall(swiotlb_create_default_debugfs);
>  
>  #endif
> +
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> +static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> + struct device *dev)
> +{
> + struct io_tlb_mem *mem = rmem->priv;
> + unsigned long nslabs = rmem->size >> IO_TLB_SHIFT;
> +
> + if (dev->dma_io_tlb_mem)
> + return 0;
> +
> + /*
> +  * Since multiple devices can share the same pool, the private data,
> +  * io_tlb_mem struct, will be initialized by the first device attached
> +  * to it.
> +  */
> + if (!mem) {
> + mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL);
> + if (!mem)
> + return -ENOMEM;
> +
> + if (PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) {
> + kfree(mem);
> + return -EINVAL;

This could probably deserve a warning here to indicate that the reserved
area must be accessible within the linear mapping as I would expect a
lot of people to trip over that.

Reviewed-by: Florian Fainelli 
-- 
Florian

Re: [PATCH v7 01/15] swiotlb: Refactor swiotlb init functions

[Florian Fainelli]

On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new function, swiotlb_init_io_tlb_mem, for the io_tlb_mem struct
> initialization to make the code reusable.
> 
> Note that we now also call set_memory_decrypted in swiotlb_init_with_tbl.
> 
> Signed-off-by: Claire Chang 
> ---
>  kernel/dma/swiotlb.c | 51 ++--
>  1 file changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 8ca7d505d61c..d3232fc19385 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -168,9 +168,30 @@ void __init swiotlb_update_mem_attributes(void)
>   memset(vaddr, 0, bytes);
>  }
>  
> -int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int 
> verbose)
> +static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t 
> start,
> + unsigned long nslabs, bool late_alloc)
>  {
> + void *vaddr = phys_to_virt(start);
>   unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
> +
> + mem->nslabs = nslabs;
> + mem->start = start;
> + mem->end = mem->start + bytes;
> + mem->index = 0;
> + mem->late_alloc = late_alloc;
> + spin_lock_init(>lock);
> + for (i = 0; i < mem->nslabs; i++) {
> + mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
> + mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
> + mem->slots[i].alloc_size = 0;
> + }
> +
> + set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
> + memset(vaddr, 0, bytes);

You are doing an unconditional set_memory_decrypted() followed by a
memset here, and then:

> +}
> +
> +int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int 
> verbose)
> +{
>   struct io_tlb_mem *mem;
>   size_t alloc_size;
>  
> @@ -186,16 +207,8 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned 
> long nslabs, int verbose)
>   if (!mem)
>   panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
> __func__, alloc_size, PAGE_SIZE);
> - mem->nslabs = nslabs;
> - mem->start = __pa(tlb);
> - mem->end = mem->start + bytes;
> - mem->index = 0;
> - spin_lock_init(>lock);
> - for (i = 0; i < mem->nslabs; i++) {
> - mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
> - mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
> - mem->slots[i].alloc_size = 0;
> - }
> +
> + swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false);

You convert this call site with swiotlb_init_io_tlb_mem() which did not
do the set_memory_decrypted()+memset(). Is this okay or should
swiotlb_init_io_tlb_mem() add an additional argument to do this
conditionally?
-- 
Florian

Re: [PATCH] Revert "ARM: dts: bcm2711: Add the BSC interrupt controller"

[Florian Fainelli]

On 2/12/2021 11:11 AM, Florian Fainelli wrote:
> As Dave reported:
> 
> This seems to have unintended side effects.  GIC interrupt 117 is shared
> between the standard I2C controllers (i2c-bcm2835) and the l2-intc block
> handling the HDMI I2C interrupts.
> 
> There is not a great way to share an interrupt between an interrupt
> controller using the chained IRQ handler which is an interrupt flow and
> another driver like i2c-bcm2835 which uses an interrupt handler
> (although it specifies IRQF_SHARED).
> 
> Simply revert this change for now which will mean that HDMI I2C will be
> polled, like it was before.
> 
> Reported-by: Dave Stevenson 
> Signed-off-by: Florian Fainelli 

Applied to devicetree/next, thanks!
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] Revert "ARM: dts: bcm2711: Add the BSC interrupt controller"

[Florian Fainelli]

As Dave reported:

This seems to have unintended side effects.  GIC interrupt 117 is shared
between the standard I2C controllers (i2c-bcm2835) and the l2-intc block
handling the HDMI I2C interrupts.

There is not a great way to share an interrupt between an interrupt
controller using the chained IRQ handler which is an interrupt flow and
another driver like i2c-bcm2835 which uses an interrupt handler
(although it specifies IRQF_SHARED).

Simply revert this change for now which will mean that HDMI I2C will be
polled, like it was before.

Reported-by: Dave Stevenson 
Signed-off-by: Florian Fainelli 
---
 arch/arm/boot/dts/bcm2711.dtsi | 12 
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 462b1dfb0385..720beec54d61 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -308,14 +308,6 @@ dvp: clock@7ef0 {
#reset-cells = <1>;
};
 
-   bsc_intr: interrupt-controller@7ef00040 {
-   compatible = "brcm,bcm2711-l2-intc", "brcm,l2-intc";
-   reg = <0x7ef00040 0x30>;
-   interrupts = ;
-   interrupt-controller;
-   #interrupt-cells = <1>;
-   };
-
aon_intr: interrupt-controller@7ef00100 {
compatible = "brcm,bcm2711-l2-intc", "brcm,l2-intc";
reg = <0x7ef00100 0x30>;
@@ -362,8 +354,6 @@ ddc0: i2c@7ef04500 {
reg = <0x7ef04500 0x100>, <0x7ef00b00 0x300>;
reg-names = "bsc", "auto-i2c";
clock-frequency = <97500>;
-   interrupt-parent = <_intr>;
-   interrupts = <0>;
status = "disabled";
};
 
@@ -405,8 +395,6 @@ ddc1: i2c@7ef09500 {
reg = <0x7ef09500 0x100>, <0x7ef05b00 0x300>;
reg-names = "bsc", "auto-i2c";
clock-frequency = <97500>;
-   interrupt-parent = <_intr>;
-   interrupts = <1>;
status = "disabled";
};
};
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 14/15] ARM: dts: bcm2711: Add the BSC interrupt controller

[Florian Fainelli]

On 2/10/21 7:49 AM, Dave Stevenson wrote:
> Hi Marc.
> 
> On Wed, 10 Feb 2021 at 15:30, Marc Zyngier  wrote:
>>
>> Hi Maxime,
>>
>> On 2021-02-10 14:40, Maxime Ripard wrote:
>>> Hi Dave,
>>>
>>> On Tue, Feb 09, 2021 at 09:49:05AM +, Dave Stevenson wrote:
 On Mon, 11 Jan 2021 at 14:23, Maxime Ripard  wrote:
>
> The BSC controllers used for the HDMI DDC have an interrupt controller
> shared between both instances. Let's add it to avoid polling.

 This seems to have unintended side effects.
 GIC interrupt 117 is shared between the standard I2C controllers
 (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.

 Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
 doesn't appear to be an option for l2-intc registering as an interrupt
 controller. i2c-bcm2835 therefore loses out and fails to register for
 the interrupt.

 Is there an equivalent flag that an interrupt controller can add to
 say that the parent interrupt is shared? Is that even supported?
>>>
>>> Indeed, it looks like setting an equivalent to IRQF_SHARED would be the
>>> solution, but I couldn't find anything that would allow us to in the
>>> irqchip code.
>>>
>>> Marc, Thomas, is it something that is allowed?
>>
>> No, not really. That's because the chained handler is actually an
>> interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong
>> level for that.
>>
>> I can see two possibilities:
>>
>> - the l2-intc gets turned into a normal handler, and does the demux
>>from there. Horrible stuff.
>>
>> - the i2c controller gets parented to the l2c-int as a fake interrupt,
>>and gets called from there. Horrible stuff.
>>
>> Pick your poison... :-/
> 
> Thanks for the info.
> 
> Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb
> blocks (which the driver supports anyway).
> HDMI I2C generally isn't heavily used once displays are connected, so
> I'd be OK with that.
> 
> (We can keep the l2-intc that handles CEC and HPD as that is on a
> unique GIC interrupt).

Agreed, Maxime or Nicolas do you want me to send a revert of this patch?
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 01/15] ARM: bcm: Select BRCMSTB_L2_IRQ for bcm2835

[Florian Fainelli]

On 1/11/2021 6:22 AM, Maxime Ripard wrote:
> The BCM2711 has a number of instances of interrupt controllers handled
> by the driver behind the BRCMSTB_L2_IRQ Kconfig option (irq-brcmstb-l2).
> 
> Let's select that driver as part of the ARCH_BCM2835 Kconfig option.
> 
> Signed-off-by: Maxime Ripard 

Acked-by: Florian Fainelli 

Nicolas, I suppose you will be taking patches 1 and 14, 15 through the
SoC pull request, right?
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dt-bindings: bcm2835-vec: Add power-domains property

[Florian Fainelli]

On 1/7/2021 6:42 PM, Rob Herring wrote:
> On Wed, 23 Dec 2020 20:24:33 +0100, Stefan Wahren wrote:
>> Adding the missing property power-domains to the bcm2835-vec schema to fix
>> the following dtbs_check issue:
>>
>> vec@7e806000: 'power-domains' does not match any of the regexes: ...
>>
>> Signed-off-by: Stefan Wahren 
>> ---
>>  Documentation/devicetree/bindings/display/brcm,bcm2835-vec.yaml | 3 +++
>>  1 file changed, 3 insertions(+)
>>
> 
> Acked-by: Rob Herring 
> 

I thought you were going to apply this directly?
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dt-bindings: bcm2835-vec: Add power-domains property

[Florian Fainelli]

On 12/23/2020 11:24 AM, Stefan Wahren wrote:
> Adding the missing property power-domains to the bcm2835-vec schema to fix
> the following dtbs_check issue:
> 
> vec@7e806000: 'power-domains' does not match any of the regexes: ...
> 
> Signed-off-by: Stefan Wahren 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] drm/v3d: Minor improvements

[Florian Fainelli]

On 12/23/2020 1:39 PM, Florian Fainelli wrote:
> 
> 
> On 12/23/2020 12:27 PM, Stefan Wahren wrote:
>> This small series of v3d patches is a preparation for the upcoming bcm2711
>> support. The bcm2711 support will be send separate, because it involves
>> bigger changes.
>>
>> I'm not sure that the schema conversion patch is sufficient.
>>
>> Patch 2,3 are directly taken from Raspberry Pi 4 vendor tree.
>>
>> Nicolas Saenz Julienne (1):
>>   drm/v3d: Use platform_get_irq_optional() to get optional IRQs
>>
>> Phil Elwell (2):
>>   drm/v3d: Set dma_mask as well as coherent_dma_mask
>>   drm/v3d: Don't clear MMU control bits on exception
> 
> You need to amend your Signed-off-by to all of those 3 patches that you
> did not author.

Looks like you fixed it in v2 about 10 minutes after, sorry for the noise.
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] drm/v3d: Minor improvements

[Florian Fainelli]

On 12/23/2020 12:27 PM, Stefan Wahren wrote:
> This small series of v3d patches is a preparation for the upcoming bcm2711
> support. The bcm2711 support will be send separate, because it involves
> bigger changes.
> 
> I'm not sure that the schema conversion patch is sufficient.
> 
> Patch 2,3 are directly taken from Raspberry Pi 4 vendor tree.
> 
> Nicolas Saenz Julienne (1):
>   drm/v3d: Use platform_get_irq_optional() to get optional IRQs
> 
> Phil Elwell (2):
>   drm/v3d: Set dma_mask as well as coherent_dma_mask
>   drm/v3d: Don't clear MMU control bits on exception

You need to amend your Signed-off-by to all of those 3 patches that you
did not author.
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/15] ARM: dts: bcm2711: Add the CEC interrupt controller

[Florian Fainelli]

On 12/10/2020 5:46 AM, Maxime Ripard wrote:
> The CEC and hotplug interrupts go through an interrupt controller shared
> between the two HDMI controllers.
> 
> Let's add that interrupt controller and the interrupts for both HDMI
> controllers
> 
> Signed-off-by: Maxime Ripard 

Reviewed-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/15] irqchip: Allow to compile bcmstb on other platforms

[Florian Fainelli]

On 12/10/2020 5:46 AM, Maxime Ripard wrote:
> The BCM2711 uses a number of instances of the bcmstb-l2 controller in its
> display engine. Let's allow the driver to be enabled through KConfig.
> 
> Signed-off-by: Maxime Ripard 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/15] ARM: dts: bcm2711: Add the BSC interrupt controller

[Florian Fainelli]

On 12/10/2020 5:46 AM, Maxime Ripard wrote:
> The BSC controllers used for the HDMI DDC have an interrupt controller
> shared between both instances. Let's add it to avoid polling.
> 
> Signed-off-by: Maxime Ripard 

Reviewed-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips

[Florian Fainelli]

On 9/7/2020 10:43 AM, Jim Quinlan wrote:

On Mon, Sep 7, 2020 at 5:16 AM Lorenzo Pieralisi
 wrote:


On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote:

On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig  wrote:


On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote:

Hi,

On 8/24/2020 12:30 PM, Jim Quinlan wrote:


Patchset Summary:
Enhance a PCIe host controller driver.  Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets.  See the 'v1' notes below for more info.


We are version 11 and counting, and it is not clear to me whether there is
any chance of getting these patches reviewed and hopefully merged for the
5.10 merge window.

There are a lot of different files being touched, so what would be the
ideal way of routing those changes towards inclusion?


FYI, I offered to take the dma-mapping bits through the dma-mapping tree.
I have a bit of a backlog, but plan to review and if Jim is ok with that
apply the current version.

Sounds good to me.


Hi Jim,

is the dependency now solved ? Should we review/take this series as
is for v5.10 through the PCI tree ?

Hello Lorenzo,

We are still working out a regression with the DMA offset commit on
the RaspberryPi.  Nicolas has found the root cause and we are now
devising a solution.


Maybe we can parallelize the PCIe driver review while the DMA changes 
are being worked on in Christoph's branch. Lorenzo, are you fine with 
the PCIe changes proper?

--
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset

[Florian Fainelli]

On 9/2/2020 3:38 PM, Nathan Chancellor wrote:
[snip]

Hello Nathan,

Can you tell me how much memory your RPI has and if all of it is


This is the 4GB version.


accessible by the PCIe device?  Could you also please include the DTS
of the PCIe node?  IIRC, the RPI firmware does some mangling of the
PCIe DT before Linux boots -- could you describe what is going on
there?


Unfortunately, I am not familiar with how to get this information. If
you could provide some instructions for how to do so, I am more than
happy to. I am not very knowleagable about the inner working of the Pi,
I mainly use it as a test platform for making sure that LLVM does not
cause problems on real devices.


Can you bring the dtc application to your Pi root filesystem, and if so, 
can you run the following:


dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb

or cat /sys/firmware/fdt > device.dtb

and attach the resulting file?




Finally, can you attach the text of the full boot log?


I have attached a working and broken boot log. Thank you for the quick
response!


Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by 
any chance?


I have a suspicion that this part of the DTS for the bcm2711.dtsi 
platform is at fault:


https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264

and the resulting dma-ranges parsing is just not working for reasons to 
be determined.

--
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips

[Florian Fainelli]

Hi,

On 8/24/2020 12:30 PM, Jim Quinlan wrote:


Patchset Summary:
   Enhance a PCIe host controller driver.  Because of its unusual design
   we are foced to change dev->dma_pfn_offset into a more general role
   allowing multiple offsets.  See the 'v1' notes below for more info.


We are version 11 and counting, and it is not clear to me whether there 
is any chance of getting these patches reviewed and hopefully merged for 
the 5.10 merge window.


There are a lot of different files being touched, so what would be the 
ideal way of routing those changes towards inclusion?


Thanks!
--
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v8 00/12] PCI: brcmstb: enable PCIe for STB chips

[Florian Fainelli]

On 7/15/20 7:35 AM, Jim Quinlan wrote:
> Patchset Summary:
>   Enhance a PCIe host controller driver.  Because of its unusual design
>   we are foced to change dev->dma_pfn_offset into a more general role
>   allowing multiple offsets.  See the 'v1' notes below for more info.

Christoph, Robin, are you happy with this version?

> 
> v8:
>   Commit: "device core: Introduce DMA range map, supplanting ..."
>   -- To satisfy a specific m68 compile configuration, I moved the 'struct
>  bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined
>  three inline functions for !CONFIG_HAS_DMA (kernel test robot).
>   -- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set
>  a pfn_offset outside of_dma_configure() but the code offers no 
>  insight on the size of the translation window.  V7 had me using
>  SIZE_MAX as the size.  I have since contacted the sunXi maintainer and
>  he said that using a size of SZ_4G would cover sunXi configurations.
> 
> v7:
>   Commit: "device core: Introduce DMA range map, supplanting ..."
>   -- remove second kcalloc/copy in device.c (AndyS)
>   -- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
>   -- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
>   -- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
>   -- Fixed compile error in "sun6i_csi.c" (kernel test robot)
>   Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
>   -- correct name of function in the commit msg (SergeiS)
>   
> v6:
>   Commit "device core: Introduce DMA range map":
>   -- of_dma_get_range() now takes a single argument and returns either
>  NULL, a valid map, or an ERR_PTR. (Robin)
>   -- offsets are no longer a PFN value but an actual address. (Robin)
>   -- the bus_dma_region struct stores the range size instead of
>  the cpu_end and pci_end values. (Robin)
>   -- devices that were setting a single offset with no boundaries
>  have been modified to have boundaries; in a few places
>  where this informatino was unavilable a /* FIXME: ... */
>  comment was added. (Robin)
>   -- dma_attach_offset_range() can be called when an offset
>  map already exists; if it's range is already present
>  nothing is done and success is returned. (Robin)
>   All commits:
>   -- Man name/style/corrections/etc changed (Bjorn)
>   -- rebase to Torvalds master
> 
> v5:
>   Commit "device core: Introduce multiple dma pfn offsets"
>   -- in of/address.c: "map_size = 0" => "*map_size = 0"
>   -- use kcalloc instead of kzalloc (AndyS)
>   -- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
>   Commit "PCI: brcmstb: Set internal memory viewport sizes"
>   -- now gives error on missing dma-ranges property.
>   Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
>   -- removed "Allof:" from brcm,scb-sizes definition (RobH)
>   All Commits:
>   -- indentation style, use max chars 100 (AndyS)
>   -- rebased to torvalds master
> 
> v4:
>   Commit "device core: Introduce multiple dma pfn offsets"
>   -- of_dma_get_range() does not take a dev param but instead
>  takes two "out" params: map and map_size.  We do this so
>  that the code that parses dma-ranges is separate from
>  the code that modifies 'dev'.   (Nicolas)
>   -- the separate case of having a single pfn offset has
>  been removed and is now processed by going through the
>  map array. (Nicolas)
>   -- move attach_uniform_dma_pfn_offset() from of/address.c to
>  dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
>   -- devm_kcalloc => devm_kzalloc (DanC)
>   -- add/fix assignment to dev->dma_pfn_offset_map for func
>  attach_uniform_dma_pfn_offset() (DanC, Nicolas)
>   -- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
>   -- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
>   -- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
>   -- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
>   Commit "of: Include a dev param in of_dma_get_range()"
>   -- this commit was sqaushed with "device core: Introduce ..."
> 
> v3:
>   Commit "device core: Introduce multiple dma pfn offsets"
>   Commit "arm: dma-mapping: Invoke dma offset func if needed"
>   -- The above two commits have been squashed.  More importantly,
>  the code has been modified so that the functionality for
>  multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
>  In fact, dma_pfn_offset is removed and supplanted by
>  dma_pfn_offset_map, which is a pointer to an array.  The
>  more common case of a uniform offset is now handled as
>  a map with a single entry, while cases requiring multiple
>  pfn offsets use a map with multiple entries.  Code paths
>  that used to do this:
> 
>  dev->dma_pfn_offset = mydrivers_pfn_offset;
> 
>  have been changed to do this:
> 
>  attach_uniform_dma_pfn_offset(dev, pfn_offset);
> 
>   Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
>   

Re: [PATCH v4 1/5] mtd: rawnand: brcmnand: rename v4 registers

[Florian Fainelli]

On 5/22/2020 5:15 AM, Álvaro Fernández Rojas wrote:
> These registers are also used on v3.3.
> 
> Signed-off-by: Álvaro Fernández Rojas 
> Reviewed-by: Miquel Raynal 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 3/5] mtd: rawnand: brcmnand: rename page sizes

[Florian Fainelli]

On 5/22/2020 5:15 AM, Álvaro Fernández Rojas wrote:
> Current pages sizes apply to controllers after v3.4
> 
> Signed-off-by: Álvaro Fernández Rojas 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 4/5] dt: bindings: brcmnand: add v2.1 and v2.2 support

[Florian Fainelli]

On 5/22/2020 5:15 AM, Álvaro Fernández Rojas wrote:
> Added brcm,brcmnand-v2.1 and brcm,brcmnand-v2.2 as possible compatible
> strings to support brcmnand controllers v2.1 and v2.2.
> 
> Signed-off-by: Álvaro Fernández Rojas 

Acked-by: Florian Fainelli 
--
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 5/5] mtd: rawnand: brcmnand: support v2.1-v2.2 controllers

[Florian Fainelli]

On 5/22/2020 5:15 AM, Álvaro Fernández Rojas wrote:
> v2.1: tested on Netgear DGND3700v1 (BCM6368)
> v2.2: tested on Netgear DGND3700v2 (BCM6362)
> 
> Signed-off-by: Álvaro Fernández Rojas 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 2/5] mtd: rawnand: brcmnand: fix CS0 layout

[Florian Fainelli]

On 5/22/2020 5:15 AM, Álvaro Fernández Rojas wrote:
> Only v3.3-v5.0 have a different CS0 layout.
> Controllers before v3.3 use the same layout for every CS.
> 
> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB 
> NAND controller")
> Signed-off-by: Álvaro Fernández Rojas 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] nand: brcmnand: support v2.1-v2.2 controllers

[Florian Fainelli]

On 5/10/2020 8:14 AM, Álvaro Fernández Rojas wrote:
> Tested on Netgear DGND3700v2 (BCM6362 with v2.2 controller).
> 
> Signed-off-by: Álvaro Fernández Rojas 

Can you fix a couple of things for your future submissions:

- for patch count > 1, please provide a cover letter introducing your
patches

- for this specific patch, you are missing a Device Tree binding
document update with the new compatible strings you are adding

Thank you!
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] nand: brcmnand: correctly verify erased pages

[Florian Fainelli]

On 5/4/2020 2:29 AM, Álvaro Fernández Rojas wrote:
> The current code checks that the whole OOB area is erased.
> This is a problem when JFFS2 cleanmarkers are added to the OOB, since it will
> fail due to the usable OOB bytes not being 0xff.
> Correct this by only checking that the ECC aren't 0xff.
> 
> Signed-off-by: Álvaro Fernández Rojas 

Can you provide a Fixes: tag for this change?

> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index e4e3ceeac38f..546f0807b887 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -2018,6 +2018,7 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, 
> struct nand_chip *chip,
>  static int brcmstb_nand_verify_erased_page(struct mtd_info *mtd,
> struct nand_chip *chip, void *buf, u64 addr)
>  {
> + struct mtd_oob_region oobecc;
>   int i, sas;
>   void *oob = chip->oob_poi;
>   int bitflips = 0;
> @@ -2035,11 +2036,24 @@ static int brcmstb_nand_verify_erased_page(struct 
> mtd_info *mtd,
>   if (ret)
>   return ret;
>  
> - for (i = 0; i < chip->ecc.steps; i++, oob += sas) {
> + for (i = 0; i < chip->ecc.steps; i++) {
>   ecc_chunk = buf + chip->ecc.size * i;
> - ret = nand_check_erased_ecc_chunk(ecc_chunk,
> -   chip->ecc.size,
> -   oob, sas, NULL, 0,
> +
> + ret = nand_check_erased_ecc_chunk(ecc_chunk, chip->ecc.size,
> +   NULL, 0, NULL, 0,
> +   chip->ecc.strength);
> + if (ret < 0)
> + return ret;
> +
> + bitflips = max(bitflips, ret);
> + }
> +
> + for (i = 0; mtd->ooblayout->ecc(mtd, i, ) != -ERANGE; i++)
> + {
> + ret = nand_check_erased_ecc_chunk(NULL, 0,
> +   oob + oobecc.offset,
> +   oobecc.length,
> +   NULL, 0,
> chip->ecc.strength);
>   if (ret < 0)
>   return ret;
> 

-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 01/91] i2c: brcmstb: Allow to compile it on BCM2835

[Florian Fainelli]

On 4/24/2020 9:13 AM, Wolfram Sang wrote:
> 
>>  config I2C_BRCMSTB
>>  tristate "BRCM Settop/DSL I2C controller"
>> -depends on ARCH_BRCMSTB || BMIPS_GENERIC || ARCH_BCM_63XX || \
>> -   COMPILE_TEST
>> +depends on ARCH_BCM2835 || ARCH_BRCMSTB || BMIPS_GENERIC || \
>> +   ARCH_BCM_63XX || COMPILE_TEST
> 
> Isn't there something like ARCH_BROADCOM which we could use here instead
> of adding each and every SoC?

If you are worried about this list growing bigger, I do not think this
is going to happen beyond this changeset (famous last words).

There is no ARCH_BROADCOM because there is typically very little
commonality between SoC architectures within various Broadcom business
units (left hand is not supposed to talk to the right hand) with the
exception of a few peripherals that have been historically shared (NAND,
SPI, XHCI, Ethernet PHYs/switches, etc. etc. This I2C controller
historically came from the STB business unit, which given the market
space has also engineered its own HDMI core and naturally incorporated
the I2C core it already had into the HDMI core. Up until 2711, that HDMI
core was not used by the 283x family at all.
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/89] dt-bindings: i2c: brcmstb: Add BCM2711 BSC/AUTO-I2C binding

[Florian Fainelli]

On 2/24/20 1:06 AM, Maxime Ripard wrote:
> The HDMI blocks in the BCM2771 have an i2c controller to retrieve the
> EDID. This block is split into two parts, the BSC and the AUTO_I2C,
> lying in two separate register areas.
> 
> The AUTO_I2C block has a mailbox-like interface and will take away the
> BSC control from the CPU if enabled. However, the BSC is the actually
> the same controller than the one supported by the brcmstb driver, and
> the AUTO_I2C doesn't really bring any immediate benefit.
> 
> We can model it in the DT as a single device with two register range,
> which will allow us to use or or the other in the driver without
> changing anything in the DT.
> 
> Cc: Kamal Dasu 
> Cc: Florian Fainelli 
> Cc: Rob Herring 
> Cc: Wolfram Sang 
> Cc: bcm-kernel-feedback-l...@broadcom.com
> Cc: linux-...@vger.kernel.org
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Maxime Ripard 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 22/89] clk: bcm: rpi: Discover the firmware clocks

[Florian Fainelli]

On 2/24/20 1:06 AM, Maxime Ripard wrote:
> The firmware has an interface to discover the clocks it exposes.
> 
> Let's use it to discover, register the clocks in the clocks framework and
> then expose them through the device tree for consumers to use them.
> 
> Cc: Michael Turquette 
> Cc: Stephen Boyd 
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Maxime Ripard 

That seems like a re-implementaiton of SCMI without all of its
protocols, without being able to use the existing drivers, maybe a
firmware update should be considered so standard drivers can be leveraged?
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/89] i2c: brcmstb: Support BCM2711 HDMI BSC controllers

[Florian Fainelli]

On 2/24/20 1:06 AM, Maxime Ripard wrote:
> The HDMI blocks in the BCM2771 have an i2c controller to retrieve the
> EDID. This block is split into two parts, the BSC and the AUTO_I2C,
> lying in two separate register areas.
> 
> The AUTO_I2C block has a mailbox-like interface and will take away the
> BSC control from the CPU if enabled. However, the BSC is the actually
> the same controller than the one supported by the brcmstb driver, and
> the AUTO_I2C doesn't really bring any immediate benefit.
> 
> Let's use the BSC then, but let's also tie the AUTO_I2C registers with a
> separate compatible so that we can enable AUTO_I2C if needed in the
> future.
> 
> The AUTO_I2C is enabled by default at boot though, so we first need to
> release the BSC from the AUTO_I2C control.
> 
> Cc: Kamal Dasu 
> Cc: Florian Fainelli 
> Cc: Wolfram Sang 
> Cc: bcm-kernel-feedback-l...@broadcom.com
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Maxime Ripard 

[snip]

> @@ -705,6 +737,7 @@ static SIMPLE_DEV_PM_OPS(brcmstb_i2c_pm, 
> brcmstb_i2c_suspend,
>  static const struct of_device_id brcmstb_i2c_of_match[] = {
>   {.compatible = "brcm,brcmstb-i2c"},
>   {.compatible = "brcm,brcmper-i2c"},
> + {.compatible = "brcm,bcm2711-hdmi-i2c"},

You could have added the bcm2711_release_bsc here as a function attached
with the of_device_id::data member of the structure and do:

if (data && data->init_func)
rc = data->init_func(dev);

But we can defer that until we have a second compatible string that
requires the same approach.

Akked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/89] dt-bindings: i2c: brcmstb: Convert the BRCMSTB binding to a schema

[Florian Fainelli]

On 2/24/20 1:06 AM, Maxime Ripard wrote:
> Switch the DT binding to a YAML schema to enable the DT validation.
> 
> Cc: Kamal Dasu 
> Cc: Florian Fainelli 
> Cc: Rob Herring 
> Cc: Wolfram Sang 
> Cc: bcm-kernel-feedback-l...@broadcom.com
> Cc: linux-...@vger.kernel.org
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Maxime Ripard 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/89] i2c: brcmstb: Allow to compile it on BCM2835

[Florian Fainelli]

On 2/24/20 1:06 AM, Maxime Ripard wrote:
> The BCM2711, supported by ARCH_BCM2835, also has a controller by the
> brcmstb driver so let's allow it to be compiled on that platform.
> 
> Cc: Kamal Dasu 
> Cc: Florian Fainelli 
> Cc: Wolfram Sang 
> Cc: bcm-kernel-feedback-l...@broadcom.com
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Maxime Ripard 

Acked-by: Florian Fainelli 
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] MIPS: c-r4k: Invalidate BMIPS5000 ZSCM prefetch lines

[Florian Fainelli]

On 2/6/2020 11:30 AM, Kamal Dasu wrote:
> Zephyr secondary cache is 256KB, 128B lines. 32B sectors. A secondary cache
> line can contain two instruction cache lines (64B), or four data cache
> lines (32B). Hardware prefetch Cache detects stream access, and prefetches
> ahead of processor access. Add support to inavalidate BMIPS5000 cpu zephyr

s/inavalidate/invalidate/

> secondary cache module (ZSCM) on DMA from device so that data returned is
> coherent during DMA read operations.

Just a few nits, see below, with those addressed:

Reviewed-by: Florian Fainelli 

> 
> Signed-off-by: Kamal Dasu 
> ---
>  arch/mips/mm/c-r4k.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
> index 5f3d0103b95d..2d8892ba68ab 100644
> --- a/arch/mips/mm/c-r4k.c
> +++ b/arch/mips/mm/c-r4k.c
> @@ -901,6 +901,35 @@ static void r4k_dma_cache_wback_inv(unsigned long addr, 
> unsigned long size)
>   __sync();
>  }
>  
> +static void prefetch_cache_inv(unsigned long addr, unsigned long size)
> +{
> + unsigned int linesz = cpu_scache_line_size();
> + unsigned long addr0 = addr, addr1;
> + int cpu_type = current_cpu_type();
> +
> + if (cpu_type == CPU_BMIPS5000) {

I would re-organize this and move this out of the prefetch_cache_inv()
such that platforms which do not require that operation can have it
optimized out, see below:

> + /* invalidate zephyr secondary cache module prefetch lines */
> + addr0 &= ~(linesz - 1);
> + addr1 = (addr0 + size - 1) & ~(linesz - 1);
> +
> + protected_writeback_scache_line(addr0);
> + if (likely(addr1 != addr0))
> + protected_writeback_scache_line(addr1);
> + else
> + return;
> +
> + addr0 += linesz;
> + if (likely(addr1 != addr0))
> + protected_writeback_scache_line(addr0);
> + else
> + return;
> +
> + addr1 -= linesz;
> + if (likely(addr1 > addr0))
> + protected_writeback_scache_line(addr0);
> + }
> +}
> +
>  static void r4k_dma_cache_inv(unsigned long addr, unsigned long size)
>  {
>   /* Catch bad driver code */
> @@ -908,6 +937,7 @@ static void r4k_dma_cache_inv(unsigned long addr, 
> unsigned long size)
>   return;
>  
>   preempt_disable();

if (current_cpu_type() == CPU_BMIPS5000)
prefetch_cache_inv(addr, size);

> + prefetch_cache_inv(addr, size);
>   if (cpu_has_inclusive_pcaches) {
>   if (size >= scache_size) {
>   if (current_cpu_type() != CPU_LOONGSON64)
> 

-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu/drm/v3d: Add ARCH_BCM2835 to DRM_V3D Kconfig

[Florian Fainelli]

On 12/18/19 6:39 AM, Nicolas Saenz Julienne wrote:
> Hi Peter,
> 
> On Wed, 2019-12-18 at 08:43 +, Peter Robinson wrote:
>> On arm64 the config ARCH_BCM doesn't exist so to be able to
>> build for platforms such as the Raspberry Pi 4 we need to add
>> ARCH_BCM2835 similar to what has been done on vc4.
>>
>> Signed-off-by: Peter Robinson 
>> ---
> 
> v3d's upstream implementation doesn't support RPi4 for now. So I don't see how
> we could benefit from this.

Right, but it should support the Pi3 running in 64-bit mode too, so
maybe that would be a better justification to put in the commit message?

> 
> That said you're more than welcome to have a go at adding support for RPi4. It
> seems to me that the divergence betweeen us and Raspberry Pi foundation's
> kernel isn't that big. Maybe Eric can share some extra light on this.
> 
> Regards,
> Nicolas
> 
>>  drivers/gpu/drm/v3d/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/v3d/Kconfig b/drivers/gpu/drm/v3d/Kconfig
>> index 9a5c44606337..b0e048697964 100644
>> --- a/drivers/gpu/drm/v3d/Kconfig
>> +++ b/drivers/gpu/drm/v3d/Kconfig
>> @@ -1,7 +1,7 @@
>>  # SPDX-License-Identifier: GPL-2.0-only
>>  config DRM_V3D
>>  tristate "Broadcom V3D 3.x and newer"
>> -depends on ARCH_BCM || ARCH_BCMSTB || COMPILE_TEST
>> +depends on ARCH_BCM || ARCH_BCMSTB || ARCH_BCM2835 || COMPILE_TEST
>>  depends on DRM
>>  depends on COMMON_CLK
>>  depends on MMU
> 


-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] Revert "ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware."

[Florian Fainelli]

On 10/9/19 2:33 PM, Stefan Wahren wrote:
> Hi Florian,
> 
> Am 23.09.19 um 20:56 schrieb Florian Fainelli:
>>
>> On 9/8/2019 8:44 AM, Stefan Wahren wrote:
>>> Since release of the new BCM2835 PM driver there has been several reports
>>> of V3D probing issues. This is caused by timeouts during powering-up the
>>> GRAFX PM domain:
>>>
>>>   bcm2835-power: Timeout waiting for grafx power OK
>>>
>>> I was able to reproduce this reliable on my Raspberry Pi 3B+ after setting
>>> force_turbo=1 in the firmware configuration. Since there are no issues
>>> using the firmware PM driver with the same setup, there must be an issue
>>> in the BCM2835 PM driver.
>>>
>>> Unfortunately there hasn't been much progress in identifying the root cause
>>> since June (mostly in the lack of documentation), so i decided to switch
>>> back until the issue in the BCM2835 PM driver is fixed.
>>>
>>> Link: https://github.com/raspberrypi/linux/issues/3046
>>> Fixes: e1dc2b2e1bef (" ARM: bcm283x: Switch V3D over to using the PM driver 
>>> instead of firmware.")
>>> Cc: sta...@vger.kernel.org
>>> Signed-off-by: Stefan Wahren 
>> Applied to devicetree/fixes, thanks!
> 
> i noticed that X hangs with recent Raspbian and Linux 5.4-rc2 after this
> revert has been applied.
> 
> Is there a chance to drop the revert?

I have not sent anything yet as I wanted to get some testing done on
other platforms, so yes, I can definitively drop that particular changes
from devicetree/fixes and not send it for a 5.4-rc3 pull request. Does
that work for you?
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/11] of: Fix DMA configuration for non-DT masters

[Florian Fainelli]

On 9/26/2019 4:20 AM, Robin Murphy wrote:
> On 2019-09-26 11:44 am, Nicolas Saenz Julienne wrote:
>> Robin, have you looked into supporting multiple dma-ranges? It's the
>> next thing
>> we need for BCM STB's PCIe. I'll have a go at it myself if nothing
>> is in
>> the
>> works already.
>
> Multiple dma-ranges as far as configuring inbound windows should work
> already other than the bug when there's any parent translation. But if
> you mean supporting multiple DMA offsets and masks per device in the
> DMA API, there's nothing in the works yet.
>>
>> Sorry, I meant supporting multiple DMA offsets[1]. I think I could
>> still make
>> it with a single DMA mask though.
> 
> The main problem for supporting that case in general is the disgusting
> carving up of the physical memory map you may have to do to guarantee
> that a single buffer allocation cannot ever span two windows with
> different offsets. I don't think we ever reached a conclusion on whether
> that was even achievable in practice.

It is with the Broadcom STB SoCs which have between 1 and 3 memory
controllers depending on the SoC, and multiple dma-ranges cells for PCIe
as a consequence.

Each memory controller has a different physical address aperture in the
CPU's physical address map (e.g.: MEMC0 is 0x0 - 0x3fff_, MEMC1
0x4000_ - 0x7_ and MEMC2 0x8000_ - 0xbfff_, not
counting the extension regions above 4GB), and while the CPU is
scheduled and arbitrated the same way across all memory controllers
(thus making it virtually UMA, almost) having a buffer span two memory
controllers would be problematic because the memory controllers do not
know how to guarantee the transaction ordering and buffer data
consistency in both DRAM itself and for other memory controller clients,
like PCIe.

We historically had to reserve the last 4KB of each memory controller to
avoid problematic controllers like EHCI to prefetch beyond the end of a
memory controller's populated memory and that also incidentally takes
care of never having a buffer cross a controller boundary. Either you
can allocate the entire buffer on a given memory controller, or you
cannot allocate memory at all on that zone/region and another one must
be found (or there is no more memory and there is a genuine OOM).

The way we reserve memory right now is based on the first patch
submitted by Jim:

https://lore.kernel.org/patchwork/patch/988469/

whereby we read the memory node's "reg" property and we map the physical
addresses to the memory controller configuration read from the specific
registers in the CPU's Bus Interface Unit (where the memory controller
apertures are architecturally defined) and then we use that to call
memblock_reserve() (not part of that patch, it should be though).
-- 
Florian

Re: [PATCH] Revert "ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware."

[Florian Fainelli]

On 9/8/2019 8:44 AM, Stefan Wahren wrote:
> Since release of the new BCM2835 PM driver there has been several reports
> of V3D probing issues. This is caused by timeouts during powering-up the
> GRAFX PM domain:
> 
>   bcm2835-power: Timeout waiting for grafx power OK
> 
> I was able to reproduce this reliable on my Raspberry Pi 3B+ after setting
> force_turbo=1 in the firmware configuration. Since there are no issues
> using the firmware PM driver with the same setup, there must be an issue
> in the BCM2835 PM driver.
> 
> Unfortunately there hasn't been much progress in identifying the root cause
> since June (mostly in the lack of documentation), so i decided to switch
> back until the issue in the BCM2835 PM driver is fixed.
> 
> Link: https://github.com/raspberrypi/linux/issues/3046
> Fixes: e1dc2b2e1bef (" ARM: bcm283x: Switch V3D over to using the PM driver 
> instead of firmware.")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Stefan Wahren 

Applied to devicetree/fixes, thanks!
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] Revert "ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware."

[Florian Fainelli]

On 9/8/19 8:44 AM, Stefan Wahren wrote:
> Since release of the new BCM2835 PM driver there has been several reports
> of V3D probing issues. This is caused by timeouts during powering-up the
> GRAFX PM domain:
> 
>   bcm2835-power: Timeout waiting for grafx power OK
> 
> I was able to reproduce this reliable on my Raspberry Pi 3B+ after setting
> force_turbo=1 in the firmware configuration. Since there are no issues
> using the firmware PM driver with the same setup, there must be an issue
> in the BCM2835 PM driver.
> 
> Unfortunately there hasn't been much progress in identifying the root cause
> since June (mostly in the lack of documentation), so i decided to switch
> back until the issue in the BCM2835 PM driver is fixed.
> 
> Link: https://github.com/raspberrypi/linux/issues/3046
> Fixes: e1dc2b2e1bef (" ARM: bcm283x: Switch V3D over to using the PM driver 
> instead of firmware.")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Stefan Wahren 

Do you want me to pick up this change directly, or would you want to
issue a pull request for 5.4-rcX with other fixes?
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32

[Florian Fainelli]

On 09/24/2018 12:04 PM, Corentin Labbe wrote:
> This patch convert meson stmmac glue driver to use all xxxsetbits32 functions.
> 
> Signed-off-by: Corentin Labbe 
> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 56 
> +-
>  1 file changed, 22 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> index c5979569fd60..abcf65588576 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "stmmac_platform.h"
>  
> @@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
>   struct clk_gate rgmii_tx_en;
>  };
>  
> -static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
> - u32 mask, u32 value)
> -{
> - u32 data;
> -
> - data = readl(dwmac->regs + reg);
> - data &= ~mask;
> - data |= (value & mask);
> -
> - writel(data, dwmac->regs + reg);
> -}

Why not make mseon8b_dwmac_mask_bits() a wrapper around
clrsetbits_le32() whose purpose is only to dereference dwmac->regs and
pass it to clrsetbits_le32()? That would be far less changes to review
and audit for correctness, same goes with every other patch in this
series touching the meson drivers.
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/vc4: Enable selection in Kconfig on any 32-bit BCM platform.

[Florian Fainelli]

On 05/09/2017 04:16 PM, Eric Anholt wrote:
> Florian Fainelli <f.faine...@gmail.com> writes:
> 
>> On 05/09/2017 11:15 AM, Eric Anholt wrote:
>>> With the Cygnus port, we needed to add at least "|| ARCH_BCM_CYGNUS"
>>> to let the module get built on a cygnus-only kernel.  However, I
>>> anticipate having a port for Kona soon, so just present the module on
>>> all of BCM.
>>>
>>> v2: Keep allowing selection with ARCH_BCM2835, since ARCH_BCM doesn't
>>> exist on arm64.
>>
>> Nit: the patch changelog usually goes after the "---" line so it gets
>> stripped with git am. Not necessary to resubmit just because of that.
> 
> Behavior on that front differs between subsystems.  DRM is one where the
> changelog is generally retained.

Once the patch lands in git, it's sort of interesting to know its
history and the context surrounding this acceptance, but there is
already so much context being lost already (like where are all other
patches from the same patch series for instance?) that I wonder if we
should not add more to it (like links to past iterations and so on).

Thanks for explaining how DRM works in that regard, though.
-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/vc4: Enable selection in Kconfig on any 32-bit BCM platform.

[Florian Fainelli]

On 05/09/2017 11:15 AM, Eric Anholt wrote:
> With the Cygnus port, we needed to add at least "|| ARCH_BCM_CYGNUS"
> to let the module get built on a cygnus-only kernel.  However, I
> anticipate having a port for Kona soon, so just present the module on
> all of BCM.
> 
> v2: Keep allowing selection with ARCH_BCM2835, since ARCH_BCM doesn't
> exist on arm64.

Nit: the patch changelog usually goes after the "---" line so it gets
stripped with git am. Not necessary to resubmit just because of that.

> 
> Signed-off-by: Eric Anholt <e...@anholt.net>
> Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch> (v1)

Acked-by: Florian Fainelli <f.faine...@gmail.com>

> ---
>  drivers/gpu/drm/vc4/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
> index 973b4203c0b2..b16aefe4a8d3 100644
> --- a/drivers/gpu/drm/vc4/Kconfig
> +++ b/drivers/gpu/drm/vc4/Kconfig
> @@ -1,6 +1,6 @@
>  config DRM_VC4
>   tristate "Broadcom VC4 Graphics"
> - depends on ARCH_BCM2835 || COMPILE_TEST
> + depends on ARCH_BCM || ARCH_BCM2835 || COMPILE_TEST
>   depends on DRM
>   depends on SND && SND_SOC
>   depends on COMMON_CLK
> 


-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vc4: Enable selection in Kconfig on any BCM platform.

[Florian Fainelli]

On 05/08/2017 11:18 AM, Eric Anholt wrote:
> With the Cygnus port, we needed to add at least "|| ARCH_BCM_CYGNUS"
> to let the module get built on a cygnus-only kernel.  However, I
> anticipate having a port for Kona soon, so just present the module on
> all of BCM.

This seems reasonable, but by replacing ARCH_BCM2835 which is common to
ARM/Linux and ARM64/Linux, you are no longer allowing an ARM64 systems
to benefit from this driver unless COMPILE_TEST is also selected, right?

This could be:

depends on COMPILE_TEST
depends on ARCH_BCM # 32-bit ARM
depends on ARCH_BCM2835 || ARCH_BCM_IPROC # 64-bit ARM

or maybe down to just:

depends on COMPILE_TEST

and let DRM, COMMON_CLK and SND drive the bulk of the dependencies?

> 
> Signed-off-by: Eric Anholt 
> ---
> 
> I would be sending this through drm-misc-next, assuming BCM
> maintainers like it.
> 
>  drivers/gpu/drm/vc4/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
> index 973b4203c0b2..f136813abf56 100644
> --- a/drivers/gpu/drm/vc4/Kconfig
> +++ b/drivers/gpu/drm/vc4/Kconfig
> @@ -1,6 +1,6 @@
>  config DRM_VC4
>   tristate "Broadcom VC4 Graphics"
> - depends on ARCH_BCM2835 || COMPILE_TEST
> + depends on ARCH_BCM || COMPILE_TEST
>   depends on DRM
>   depends on SND && SND_SOC
>   depends on COMMON_CLK
> 


-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.

[Florian Fainelli]

On 04/18/2017 04:38 PM, Eric Anholt wrote:
> For the Raspberry Pi's bindings, the power domain also implicitly
> turns on the clock and deasserts reset, but for the new Cygnus port we
> start representing the clock in the devicetree.
> 
> v2: Document the clock-names property, check for -ENOENT for no clock
> in DT.
> 
> Signed-off-by: Eric Anholt 
> ---

> + if (v3d->clk)
> + clk_disable_unprepare(v3d->clk);

The clock API allows you to pass a NULL clk and do nothing in these
cases which is what you seem to have done a few lines below, you could
simplify these checks?

> +
>   return 0;
>  }
>  
> @@ -318,6 +322,13 @@ static int vc4_v3d_runtime_resume(struct device *dev)
>   if (ret)
>   return ret;
>  
> + if (v3d->clk) {
> + int ret = clk_prepare_enable(v3d->clk);
> +
> + if (ret != 0)
> + return ret;
> + }
> +
>   vc4_v3d_init_hw(vc4->dev);
>   vc4_irq_postinstall(vc4->dev);
>  
> @@ -348,15 +359,40 @@ static int vc4_v3d_bind(struct device *dev, struct 
> device *master, void *data)
>   vc4->v3d = v3d;
>   v3d->vc4 = vc4;
>  
> + v3d->clk = devm_clk_get(dev, "v3d_clk");
> + if (IS_ERR(v3d->clk)) {
> + int ret = PTR_ERR(v3d->clk);
> +
> + if (ret == -ENOENT) {
> + /* bcm2835 didn't have a clock reference in the DT. */
> + ret = 0;
> + v3d->clk = NULL;
> + } else {
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Failed to get V3D clock: %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
>   if (V3D_READ(V3D_IDENT0) != V3D_EXPECTED_IDENT0) {
>   DRM_ERROR("V3D_IDENT0 read 0x%08x instead of 0x%08x\n",
> V3D_READ(V3D_IDENT0), V3D_EXPECTED_IDENT0);
>   return -EINVAL;
>   }
>  
> + if (v3d->clk) {
> + ret = clk_prepare_enable(v3d->clk);
> + if (ret != 0)
> + return ret;
> + }
> +
>   ret = vc4_allocate_bin_bo(drm);
> - if (ret)
> + if (ret) {
> + if (v3d->clk)
> + clk_disable_unprepare(v3d->clk);
>   return ret;
> + }
>  
>   /* Reset the binner overflow address/size at setup, to be sure
>* we don't reuse an old one.
> 


-- 
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RESEND PATCH v2 4/7] drm/vc4: Add support for the VEC (Video Encoder) IP

[Florian Fainelli]

On 12/02/2016 05:48 AM, Boris Brezillon wrote:
> The VEC IP is a TV DAC, providing support for PAL and NTSC standards.
> 
> Signed-off-by: Boris Brezillon 
> ---

> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> new file mode 100644
> index ..2d4256fcc6f2
> --- /dev/null
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -0,0 +1,657 @@
> +/*
> + * Copyright (C) 2016 Broadcom Limited

The standard copyright template post acquisition is just Broadcom, not
Broadcom Limited, nor Broadcom corporation. Can you audit your entire
submission and fix this up accordingly?

Thanks!
-- 
Florian

Re: Update on the CEC API

[Florian Fainelli]

Hi Hans,

On Thursday 27 September 2012 16:33:30 Hans Verkuil wrote:
 Hi all,
 
 During the Linux Plumbers Conference we (i.e. V4L2 and DRM developers) had a
 discussion on how to handle the CEC protocol that's part of the HDMI 
standard.
 
 The decision was made to create a CEC bus with CEC clients, each represented
 by a /dev/cecX device. So this is independent of the V4L or DRM APIs. 
 
 In addition interested subsystems (alsa for the Audio Return Channel, and
 possibly networking as well for ethernet over HDMI) can intercept/send CEC
 messages as well if needed. Particularly for the CEC additions made in
 HDMI 1.4a it is no longer possible to handle the CEC protocol completely in
 userspace, but part of the intelligence has to be moved to kernelspace.

What kind of intelligence are you talking about? I see nothing in HDMI 1.4a 
or earlier that requires doing stuff in kernelspace besides managing control to 
the hardware, but I might be missing something.

In my opinion ARC is just a control mechanism, and can be dealt with in user-
space, since you really want to just have hints about when ARC is 
enabled/disabled to take appropriate actions on the audio outputs or your 
system.

 
 I've started working on this API but I am still at the stage of playing
 around with it and thinking about the best way this functionality should
 be exposed. At least I managed to get the first CEC packets transferred
 today :-)
 
 It will probably be a few weeks before I post something, but in the meantime
 if you want to use CEC and have certain requirements that need to be met,
 please let me know. If only so that I can be certain I haven't forgotten
 anything.

Here is my wish-list, if I may:
- allow for a CEC adapter to be in detached / attached mode, particularly 
useful if the hardware doing CEC can process a basic set of messages to act a 
a global wake-up source for the system

- allow for a CEC adapter to define several receive modes: unicast and 
promiscuous, which is useful for dumping the CEC bus messages

- make the CEC adapter API asynchronous for the data path, so it is easy for a 
driver to report completion of a successfully transmitted/received CEC frame

Thank you!
--
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Update on the CEC API

[Florian Fainelli]

On Monday 08 October 2012 17:49:00 Hans Verkuil wrote:
 On Mon October 8 2012 17:06:20 Florian Fainelli wrote:
  Hi Hans,
  
  On Thursday 27 September 2012 16:33:30 Hans Verkuil wrote:
   Hi all,
   
   During the Linux Plumbers Conference we (i.e. V4L2 and DRM developers) 
   had a
   discussion on how to handle the CEC protocol that's part of the HDMI 
  standard.
   
   The decision was made to create a CEC bus with CEC clients, each 
   represented
   by a /dev/cecX device. So this is independent of the V4L or DRM APIs. 
   
   In addition interested subsystems (alsa for the Audio Return Channel, and
   possibly networking as well for ethernet over HDMI) can intercept/send CEC
   messages as well if needed. Particularly for the CEC additions made in
   HDMI 1.4a it is no longer possible to handle the CEC protocol completely 
   in
   userspace, but part of the intelligence has to be moved to kernelspace.
  
  What kind of intelligence are you talking about? I see nothing in HDMI 
  1.4a 
  or earlier that requires doing stuff in kernelspace besides managing 
  control to 
  the hardware, but I might be missing something.
 
 Most notably: handling the new hotplug message. That's something that kernel
 drivers need to know. Some ARC messages might be relevant for ALSA drivers as
 well, but I need to look into those more carefully.
 
 Also remote control messages might optionally be handled through an input 
 driver.

Ok, then maybe just stick to the standard CEC_UI_* key codes, and let people
having proprietary UI functions do the rest in user-space, or write their own
input driver.

 
  In my opinion ARC is just a control mechanism, and can be dealt with in 
  user-
  space, since you really want to just have hints about when ARC is 
  enabled/disabled to take appropriate actions on the audio outputs or your 
  system.
  
   
   I've started working on this API but I am still at the stage of playing
   around with it and thinking about the best way this functionality should
   be exposed. At least I managed to get the first CEC packets transferred
   today :-)
   
   It will probably be a few weeks before I post something, but in the 
   meantime
   if you want to use CEC and have certain requirements that need to be met,
   please let me know. If only so that I can be certain I haven't forgotten
   anything.
  
  Here is my wish-list, if I may:
  - allow for a CEC adapter to be in detached / attached mode, 
  particularly 
  useful if the hardware doing CEC can process a basic set of messages to act 
  a 
  a global wake-up source for the system
 
 I have hardware that can do that, so I want to look into supporting this.
 
  - allow for a CEC adapter to define several receive modes: unicast and 
  promiscuous, which is useful for dumping the CEC bus messages
 
 I don't think I have hardware for that, but it shouldn't be difficult to add.

Definitively not, just let a driver writer specify a flag to advertise this
capability and have a getter/setter to specify the receive mode.

 
  - make the CEC adapter API asynchronous for the data path, so it is easy 
  for a 
  driver to report completion of a successfully transmitted/received CEC frame
 
 Already done.

Great, thanks!
--
Florian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Update on the CEC API

[Florian Fainelli]

On Monday 08 October 2012 17:49:00 Hans Verkuil wrote:
> On Mon October 8 2012 17:06:20 Florian Fainelli wrote:
> > Hi Hans,
> > 
> > On Thursday 27 September 2012 16:33:30 Hans Verkuil wrote:
> > > Hi all,
> > > 
> > > During the Linux Plumbers Conference we (i.e. V4L2 and DRM developers) 
> > > had a
> > > discussion on how to handle the CEC protocol that's part of the HDMI 
> > standard.
> > > 
> > > The decision was made to create a CEC bus with CEC clients, each 
> > > represented
> > > by a /dev/cecX device. So this is independent of the V4L or DRM APIs. 
> > > 
> > > In addition interested subsystems (alsa for the Audio Return Channel, and
> > > possibly networking as well for ethernet over HDMI) can intercept/send CEC
> > > messages as well if needed. Particularly for the CEC additions made in
> > > HDMI 1.4a it is no longer possible to handle the CEC protocol completely 
> > > in
> > > userspace, but part of the intelligence has to be moved to kernelspace.
> > 
> > What kind of "intelligence" are you talking about? I see nothing in HDMI 
> > 1.4a 
> > or earlier that requires doing stuff in kernelspace besides managing 
> > control to 
> > the hardware, but I might be missing something.
> 
> Most notably: handling the new hotplug message. That's something that kernel
> drivers need to know. Some ARC messages might be relevant for ALSA drivers as
> well, but I need to look into those more carefully.
> 
> Also remote control messages might optionally be handled through an input 
> driver.

Ok, then maybe just stick to the standard CEC_UI_* key codes, and let people
having proprietary UI functions do the rest in user-space, or write their own
input driver.

> 
> > In my opinion ARC is just a control mechanism, and can be dealt with in 
> > user-
> > space, since you really want to just have hints about when ARC is 
> > enabled/disabled to take appropriate actions on the audio outputs or your 
> > system.
> > 
> > > 
> > > I've started working on this API but I am still at the stage of playing
> > > around with it and thinking about the best way this functionality should
> > > be exposed. At least I managed to get the first CEC packets transferred
> > > today :-)
> > > 
> > > It will probably be a few weeks before I post something, but in the 
> > > meantime
> > > if you want to use CEC and have certain requirements that need to be met,
> > > please let me know. If only so that I can be certain I haven't forgotten
> > > anything.
> > 
> > Here is my wish-list, if I may:
> > - allow for a CEC adapter to be in "detached" / "attached" mode, 
> > particularly 
> > useful if the hardware doing CEC can process a basic set of messages to act 
> > a 
> > a global wake-up source for the system
> 
> I have hardware that can do that, so I want to look into supporting this.
> 
> > - allow for a CEC adapter to define several receive modes: unicast and 
> > "promiscuous", which is useful for dumping the CEC bus messages
> 
> I don't think I have hardware for that, but it shouldn't be difficult to add.

Definitively not, just let a driver writer specify a flag to advertise this
capability and have a getter/setter to specify the receive mode.

> 
> > - make the CEC adapter API asynchronous for the data path, so it is easy 
> > for a 
> > driver to report completion of a successfully transmitted/received CEC frame
> 
> Already done.

Great, thanks!
--
Florian

Update on the CEC API

[Florian Fainelli]

Hi Hans,

On Thursday 27 September 2012 16:33:30 Hans Verkuil wrote:
> Hi all,
> 
> During the Linux Plumbers Conference we (i.e. V4L2 and DRM developers) had a
> discussion on how to handle the CEC protocol that's part of the HDMI 
standard.
> 
> The decision was made to create a CEC bus with CEC clients, each represented
> by a /dev/cecX device. So this is independent of the V4L or DRM APIs. 
> 
> In addition interested subsystems (alsa for the Audio Return Channel, and
> possibly networking as well for ethernet over HDMI) can intercept/send CEC
> messages as well if needed. Particularly for the CEC additions made in
> HDMI 1.4a it is no longer possible to handle the CEC protocol completely in
> userspace, but part of the intelligence has to be moved to kernelspace.

What kind of "intelligence" are you talking about? I see nothing in HDMI 1.4a 
or earlier that requires doing stuff in kernelspace besides managing control to 
the hardware, but I might be missing something.

In my opinion ARC is just a control mechanism, and can be dealt with in user-
space, since you really want to just have hints about when ARC is 
enabled/disabled to take appropriate actions on the audio outputs or your 
system.

> 
> I've started working on this API but I am still at the stage of playing
> around with it and thinking about the best way this functionality should
> be exposed. At least I managed to get the first CEC packets transferred
> today :-)
> 
> It will probably be a few weeks before I post something, but in the meantime
> if you want to use CEC and have certain requirements that need to be met,
> please let me know. If only so that I can be certain I haven't forgotten
> anything.

Here is my wish-list, if I may:
- allow for a CEC adapter to be in "detached" / "attached" mode, particularly 
useful if the hardware doing CEC can process a basic set of messages to act a 
a global wake-up source for the system

- allow for a CEC adapter to define several receive modes: unicast and 
"promiscuous", which is useful for dumping the CEC bus messages

- make the CEC adapter API asynchronous for the data path, so it is easy for a 
driver to report completion of a successfully transmitted/received CEC frame

Thank you!
--
Florian