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

2022-09-21 Thread Maxime Ripard 
Hi,

On Tue, Sep 20, 2022 at 06:01:08PM +0200, Stefan Wahren wrote:
> Am 20.09.22 um 14:50 schrieb Maxime Ripard:
> > 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.
> 
> recently as i reviewed the clk-raspberrypi i noticed this, too. But from my
> point of view the clock ids should go to include/dt-bindings/clock (like
> bcm2835.h) because these clock ids are actually referenced in the DTS files
> and we need to make sure they are in sync. AFAIR this would also result in
> change from enum to defines.
> 
> Sorry, i didn't had the time to send a patch for this.

IMO, we need both, and this enum still belongs in the firmware header.
We have two separate things, the firmware interface and the DT
interface. The kernel is a consumer for both, but the fact that they
match is an implementation detail. It might even change in the future
for all we know.

So having a header to use defines for the clock indices in the DT looks
like a good idea to me, but I think we should keep that enum in the
firmware header as well.

Maxime


signature.asc
Description: PGP signature

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

2022-09-20 Thread Stefan Wahren 

Hi Maxime,

Am 20.09.22 um 14:50 schrieb Maxime Ripard:

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.


recently as i reviewed the clk-raspberrypi i noticed this, too. But from 
my point of view the clock ids should go to include/dt-bindings/clock 
(like bcm2835.h) because these clock ids are actually referenced in the 
DTS files and we need to make sure they are in sync. AFAIR this would 
also result in change from enum to defines.


Sorry, i didn't had the time to send a patch for this.



Signed-off-by: Maxime Ripard 

diff --git a/drivers/clk/bcm/clk-raspberrypi.c 
b/drivers/clk/bcm/clk-raspberrypi.c
index 876b37b8683c..1f5e6a1554e6 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -18,24 +18,6 @@
  
  #include 
  
-enum rpi_firmware_clk_id {

-   RPI_FIRMWARE_EMMC_CLK_ID = 1,
-   RPI_FIRMWARE_UART_CLK_ID,
-   RPI_FIRMWARE_ARM_CLK_ID,
-   RPI_FIRMWARE_CORE_CLK_ID,
-   RPI_FIRMWARE_V3D_CLK_ID,
-   RPI_FIRMWARE_H264_CLK_ID,
-   RPI_FIRMWARE_ISP_CLK_ID,
-   RPI_FIRMWARE_SDRAM_CLK_ID,
-   RPI_FIRMWARE_PIXEL_CLK_ID,
-   RPI_FIRMWARE_PWM_CLK_ID,
-   RPI_FIRMWARE_HEVC_CLK_ID,
-   RPI_FIRMWARE_EMMC2_CLK_ID,
-   RPI_FIRMWARE_M2MC_CLK_ID,
-   RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
-   RPI_FIRMWARE_NUM_CLK_ID,
-};
-
  static char *rpi_firmware_clk_names[] = {
[RPI_FIRMWARE_EMMC_CLK_ID]  = "emmc",
[RPI_FIRMWARE_UART_CLK_ID]  = "uart",
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h 
b/include/soc/bcm2835/raspberrypi-firmware.h
index 63426082bcb9..74c7bcc1ac2a 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -136,6 +136,24 @@ enum rpi_firmware_property_tag {
RPI_FIRMWARE_GET_DMA_CHANNELS =   0x00060001,
  };
  
+enum rpi_firmware_clk_id {

+   RPI_FIRMWARE_EMMC_CLK_ID = 1,
+   RPI_FIRMWARE_UART_CLK_ID,
+   RPI_FIRMWARE_ARM_CLK_ID,
+   RPI_FIRMWARE_CORE_CLK_ID,
+   RPI_FIRMWARE_V3D_CLK_ID,
+   RPI_FIRMWARE_H264_CLK_ID,
+   RPI_FIRMWARE_ISP_CLK_ID,
+   RPI_FIRMWARE_SDRAM_CLK_ID,
+   RPI_FIRMWARE_PIXEL_CLK_ID,
+   RPI_FIRMWARE_PWM_CLK_ID,
+   RPI_FIRMWARE_HEVC_CLK_ID,
+   RPI_FIRMWARE_EMMC2_CLK_ID,
+   RPI_FIRMWARE_M2MC_CLK_ID,
+   RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
+   RPI_FIRMWARE_NUM_CLK_ID,
+};
+
  #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
  int rpi_firmware_property(struct rpi_firmware *fw,
  u32 tag, void *data, size_t len);

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

2022-09-20 Thread Maxime Ripard 
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 

diff --git a/drivers/clk/bcm/clk-raspberrypi.c 
b/drivers/clk/bcm/clk-raspberrypi.c
index 876b37b8683c..1f5e6a1554e6 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -18,24 +18,6 @@
 
 #include 
 
-enum rpi_firmware_clk_id {
-   RPI_FIRMWARE_EMMC_CLK_ID = 1,
-   RPI_FIRMWARE_UART_CLK_ID,
-   RPI_FIRMWARE_ARM_CLK_ID,
-   RPI_FIRMWARE_CORE_CLK_ID,
-   RPI_FIRMWARE_V3D_CLK_ID,
-   RPI_FIRMWARE_H264_CLK_ID,
-   RPI_FIRMWARE_ISP_CLK_ID,
-   RPI_FIRMWARE_SDRAM_CLK_ID,
-   RPI_FIRMWARE_PIXEL_CLK_ID,
-   RPI_FIRMWARE_PWM_CLK_ID,
-   RPI_FIRMWARE_HEVC_CLK_ID,
-   RPI_FIRMWARE_EMMC2_CLK_ID,
-   RPI_FIRMWARE_M2MC_CLK_ID,
-   RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
-   RPI_FIRMWARE_NUM_CLK_ID,
-};
-
 static char *rpi_firmware_clk_names[] = {
[RPI_FIRMWARE_EMMC_CLK_ID]  = "emmc",
[RPI_FIRMWARE_UART_CLK_ID]  = "uart",
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h 
b/include/soc/bcm2835/raspberrypi-firmware.h
index 63426082bcb9..74c7bcc1ac2a 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -136,6 +136,24 @@ enum rpi_firmware_property_tag {
RPI_FIRMWARE_GET_DMA_CHANNELS =   0x00060001,
 };
 
+enum rpi_firmware_clk_id {
+   RPI_FIRMWARE_EMMC_CLK_ID = 1,
+   RPI_FIRMWARE_UART_CLK_ID,
+   RPI_FIRMWARE_ARM_CLK_ID,
+   RPI_FIRMWARE_CORE_CLK_ID,
+   RPI_FIRMWARE_V3D_CLK_ID,
+   RPI_FIRMWARE_H264_CLK_ID,
+   RPI_FIRMWARE_ISP_CLK_ID,
+   RPI_FIRMWARE_SDRAM_CLK_ID,
+   RPI_FIRMWARE_PIXEL_CLK_ID,
+   RPI_FIRMWARE_PWM_CLK_ID,
+   RPI_FIRMWARE_HEVC_CLK_ID,
+   RPI_FIRMWARE_EMMC2_CLK_ID,
+   RPI_FIRMWARE_M2MC_CLK_ID,
+   RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
+   RPI_FIRMWARE_NUM_CLK_ID,
+};
+
 #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
 int rpi_firmware_property(struct rpi_firmware *fw,
  u32 tag, void *data, size_t len);

-- 
b4 0.10.0