Skip to contentSkip to conversation

Simplify mediatek clk driver probes

PATCH In review Stephen Boyd 5 participants 19 messages Nov 6, 2018 – Nov 30, 2018

Subsystems (3)
Arm MediaTek Common clk Device tree
lore
Share & tools
lore

Stephen Boyd

author maintainer
#0 lore raw

cover Simplify mediatek clk driver probes

I suggested this some time ago but nobody has gotten around to doing it so far. Well now I have! Here's a patch series that cuts down the boiler-plate code that Mediatek clk drivers have to multiplex probe amongst the device match data.

Rob/Frank, I'd prefer to take the first patch via clk tree so I can merge it all in the next release. Otherwise, I can pull a stable commit and merge it that way in case you want to take it through your tree.

Cc Ryder Lee <[email protected]>
Cc Rob Herring <[email protected]>
Cc Frank Rowand <[email protected]>

Stephen Boyd (4):
of/device: Add a way to probe drivers by match data
clk: mediatek: Convert to platform_driver_probe_by_match_data()
clk: mediatek: Drop THIS_MODULE from platform_driver
clk: mediatek: Simplify single driver probes

 drivers/clk/mediatek/clk-mt2701-g3d.c | 28 +++---------------------
 drivers/clk/mediatek/clk-mt2701.c     | 20 +----------------
 drivers/clk/mediatek/clk-mt2712.c     | 21 +-----------------
 drivers/clk/mediatek/clk-mt6797.c     | 20 +----------------
 drivers/clk/mediatek/clk-mt7622-aud.c | 28 +++---------------------
 drivers/clk/mediatek/clk-mt7622-eth.c | 20 +----------------
 drivers/clk/mediatek/clk-mt7622-hif.c | 20 +----------------
 drivers/clk/mediatek/clk-mt7622.c     | 20 +----------------
 drivers/of/device.c                   | 31 +++++++++++++++++++++++++++
 include/linux/of_device.h             |  1 +
 10 files changed, 44 insertions(+), 165 deletions(-)
Signature · 1 line
Sent by a computer through tubes
author maintainer
#1 lore raw

PATCH 1/4 of/device: Add a way to probe drivers by match data

We have a handful of clk drivers that have a collection of slightly variant device support keyed off of the compatible string. In each of these drivers, we demux the variant and then call the "real" probe function based on whatever is stored in the match data for that compatible string. Let's generalize this function so that it can be re-used as the platform_driver probe function directly.

Cc Ryder Lee <[email protected]>
Cc Rob Herring <[email protected]>
Cc Frank Rowand <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/of/device.c       | 31 +++++++++++++++++++++++++++++++
 include/linux/of_device.h |  1 +
 2 files changed, 32 insertions(+)
drivers/of/device.c +31 -0
index 0f27fad9fe94..8381f33ed2d8 100644
@@ -195,6 +195,37 @@ const void *of_device_get_match_data(const struct device *dev)
 }
 EXPORT_SYMBOL(of_device_get_match_data);
 
+/**
+ * platform_driver_probe_by_of_match_data - Probe a platform device using match data
+ * @pdev: platform device to probe
+ *
+ * For use by device drivers that multiplex their probe function through DT
+ * match data. Drivers can use this function to call their platform
+ * device probe directly without having to implement a wrapper function.
+ *
+ * static const struct of_device_id probe_funcs[] = {
+ *      { .compatible = "compat,foo", .data = foo_probe },
+ *      {}
+ * };
+ *
+ * struct platform_driver foo_driver = {
+ *      .probe = platform_driver_probe_by_of_match_data,
+ *      .driver = {
+ *            of_match_table = probe_funcs,
+ *      },
+ * };
+ *
+ */
+int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
+{
+	int (*probe_func)(struct platform_device *pdev);
+
+	probe_func = of_device_get_match_data(&pdev->dev);
+
+	return probe_func(pdev);
+}
+EXPORT_SYMBOL_GPL(platform_driver_probe_by_of_match_data);
+
 static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
 {
 	const char *compat;
include/linux/of_device.h +1 -0
index 8d31e39dd564..4de84691d1c6 100644
@@ -33,6 +33,7 @@ extern int of_device_add(struct platform_device *pdev);
 extern int of_device_register(struct platform_device *ofdev);
 extern void of_device_unregister(struct platform_device *ofdev);
 
+extern int platform_driver_probe_by_of_match_data(struct platform_device *pdev);
 extern const void *of_device_get_match_data(const struct device *dev);
 
 extern ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len);
Signature · 1 line
Sent by a computer through tubes
#2 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoted 7 lines · Stephen Boyd

We have a handful of clk drivers that have a collection of slightly variant device support keyed off of the compatible string. In each of these drivers, we demux the variant and then call the "real" probe function based on whatever is stored in the match data for that compatible string. Let's generalize this function so that it can be re-used as the platform_driver probe function directly.

This looks really hacky to me. It sounds kind of general, but really only works if we have match data that's a single function and we lose any type checking on the function. What about things other than platform devices?

Rob

author maintainer
#3 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoting Rob Herring (2018-11-06 12:44:52)

Quoted 9 lines · Stephen Boyd
Quoted 7 lines

We have a handful of clk drivers that have a collection of slightly variant device support keyed off of the compatible string. In each of these drivers, we demux the variant and then call the "real" probe function based on whatever is stored in the match data for that compatible string. Let's generalize this function so that it can be re-used as the platform_driver probe function directly.

Quoted 3 lines

This looks really hacky to me. It sounds kind of general, but really only works if we have match data that's a single function and we lose any type checking on the function.

I don't know what this means. Are you saying that we lose the ability to type check the function pointer stored in the data member? I don't have a good answer for this besides it's not any worse than the status quo for the mediatek drivers.

One alternative is to add a DT only array to the platform_driver struct that has the platform driver probe function type and matches the index in the of_device_id table. Then we can add some logic into platform_drv_probe() to look for this table being set and find the probe function to call. Then we still have match data for anything that wants that (maybe it could be passed in to the probe function) at the cost of having another array. I don't have a use-case for this right now so I'm not sure this is a great idea.

  struct of_platform_driver_probe_func {
  	int (*probe)(struct platform_device *pdev);
  };

struct of_platform_driver_probe_func mtk_probes[] = {
mtk_probe1,

	mtk_probe2,
	mtk_probe3,

};

struct platform_driver mtk_driver = {
.of_probes = &mtk_probes;

  	.driver = {
		.name = "mtk-foo";
		.of_match_table = mtk_match_table,
	},

};

Quoted 3 lines

What about things other than
platform devices?

I suppose those would need similar functions that take the right struct type and match the driver probe function signature. To make the above more generic we could try to figure out a way to put the 'of_probes' array inside struct device_driver. That would require dropping platform_device specifically from the probe functions and having those take a plain 'struct device' that those probe functions turn into the appropriate structure with to_platform_device() or to_i2c_client()?

So the example would become

  struct of_driver_probe_func {
  	int (*probe)(struct device *dev);
  };

struct of_driver_probe_func mtk_probes[] = {
mtk_probe1,

	mtk_probe2,
	mtk_probe3,

};

struct platform_driver mtk_driver = {

  	.driver = {
		.name = "mtk-foo";
		.of_match_table = mtk_match_table,
		.of_probes = &mtk_probes;
	},

};

And the probe functions might need to container_of() the device pointer to get the struct they know they need. The probe function could also be added to of_device_id and then we would have to look and see if that pointer is populated when the device is matched in generic device code.

#4 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Hi Stephen,

Quoted 9 lines · Stephen Boyd

Quoting Rob Herring (2018-11-06 12:44:52)

Quoted 8 lines · Stephen Boyd
Quoted 6 lines

We have a handful of clk drivers that have a collection of slightly variant device support keyed off of the compatible string. In each of these drivers, we demux the variant and then call the "real" probe function based on whatever is stored in the match data for that compatible string. Let's generalize this function so that it can be re-used as the platform_driver probe function directly.

Quoted 8 lines
Quoted 3 lines

This looks really hacky to me. It sounds kind of general, but really only works if we have match data that's a single function and we lose any type checking on the function.

I don't know what this means. Are you saying that we lose the ability to type check the function pointer stored in the data member? I don't have a good answer for this besides it's not any worse than the status quo for the mediatek drivers.

The .data field has always been void *, and is used for storing different things, depending on the driver.
It's already up to the driver writer to get that right.

Quoted 19 lines

One alternative is to add a DT only array to the platform_driver struct that has the platform driver probe function type and matches the index in the of_device_id table. Then we can add some logic into platform_drv_probe() to look for this table being set and find the probe function to call. Then we still have match data for anything that wants that (maybe it could be passed in to the probe function) at the cost of having another array. I don't have a use-case for this right now so I'm not sure this is a great idea.

  struct of_platform_driver_probe_func {
        int (*probe)(struct platform_device *pdev);
  };

  struct of_platform_driver_probe_func mtk_probes[] = {
        mtk_probe1,
        mtk_probe2,
        mtk_probe3,
  };
Quoted 7 lines
  struct platform_driver mtk_driver = {
        .of_probes = &mtk_probes;
        .driver = {
                .name = "mtk-foo";
                .of_match_table = mtk_match_table,
        },
  };

This looks worse to me: people tend to be very good at keeping multiple arrays in sync :-(

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#5 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoting Geert Uytterhoeven (2018-11-09 01:56:01)

Quoted 20 lines · Stephen Boyd
Quoted 5 lines

Quoting Rob Herring (2018-11-06 12:44:52)

Quoted 1 line · Stephen Boyd
        int (*probe)(struct platform_device *pdev);

};

Quoted 13 lines
  struct of_platform_driver_probe_func mtk_probes[] = {
        mtk_probe1,
        mtk_probe2,
        mtk_probe3,
  };

  struct platform_driver mtk_driver = {
        .of_probes = &mtk_probes;
        .driver = {
                .name = "mtk-foo";
                .of_match_table = mtk_match_table,
        },
  };
Quoted 2 lines

This looks worse to me: people tend to be very good at keeping multiple arrays in sync :-(

To be _not_ very good? Agreed, and so specifying the probe function as another member in struct of_device_id seems to be the way to go.

#6 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Hi Stephen,

Quoted 21 lines · Stephen Boyd

Quoting Geert Uytterhoeven (2018-11-09 01:56:01)

Quoted 20 lines · Stephen Boyd
Quoted 18 lines

Quoting Rob Herring (2018-11-06 12:44:52)

Quoted 1 line · Stephen Boyd
        int (*probe)(struct platform_device *pdev);

};

  struct of_platform_driver_probe_func mtk_probes[] = {
        mtk_probe1,
        mtk_probe2,
        mtk_probe3,
  };

  struct platform_driver mtk_driver = {
        .of_probes = &mtk_probes;
        .driver = {
                .name = "mtk-foo";
                .of_match_table = mtk_match_table,
        },
  };
Quoted 5 lines
Quoted 2 lines

This looks worse to me: people tend to be very good at keeping multiple arrays in sync :-(

To be _not_ very good? Agreed, and so specifying the probe function as another member in struct of_device_id seems to be the way to go.

Correct, sometimes sarcasm doesn't arrive well at the other end of the electronic tunnel...

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#7 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

In reply to Stephen Boyd

Quoting Stephen Boyd (2018-11-07 10:37:31)

Quoted 22 lines

appropriate structure with to_platform_device() or to_i2c_client()?

So the example would become

  struct of_driver_probe_func {
        int (*probe)(struct device *dev);
  };

  struct of_driver_probe_func mtk_probes[] = {
        mtk_probe1,
        mtk_probe2,
        mtk_probe3,
  };

  struct platform_driver mtk_driver = {
        .driver = {
                .name = "mtk-foo";
                .of_match_table = mtk_match_table,
                .of_probes = &mtk_probes;
        },
  };
Quoted 5 lines

And the probe functions might need to container_of() the device pointer to get the struct they know they need. The probe function could also be added to of_device_id and then we would have to look and see if that pointer is populated when the device is matched in generic device code.

I guess I'll go down the path of extending the of_device_id structure?

#8 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoted 24 lines · Stephen Boyd

Quoting Stephen Boyd (2018-11-07 10:37:31)

Quoted 22 lines

appropriate structure with to_platform_device() or to_i2c_client()?

So the example would become

  struct of_driver_probe_func {
        int (*probe)(struct device *dev);
  };

  struct of_driver_probe_func mtk_probes[] = {
        mtk_probe1,
        mtk_probe2,
        mtk_probe3,
  };

  struct platform_driver mtk_driver = {
        .driver = {
                .name = "mtk-foo";
                .of_match_table = mtk_match_table,
                .of_probes = &mtk_probes;
        },
  };
Quoted 7 lines
Quoted 5 lines

And the probe functions might need to container_of() the device pointer to get the struct they know they need. The probe function could also be added to of_device_id and then we would have to look and see if that pointer is populated when the device is matched in generic device code.

I guess I'll go down the path of extending the of_device_id structure?

Unfortunately, I don't think you can change of_device_id as it's part of the kernel ABI.

Rob

author maintainer
#9 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoting Rob Herring (2018-11-29 17:01:54)

Quoted 31 lines · Stephen Boyd
Quoted 30 lines

Quoting Stephen Boyd (2018-11-07 10:37:31)

Quoted 27 lines

appropriate structure with to_platform_device() or to_i2c_client()?

So the example would become

  struct of_driver_probe_func {
        int (*probe)(struct device *dev);
  };

  struct of_driver_probe_func mtk_probes[] = {
        mtk_probe1,
        mtk_probe2,
        mtk_probe3,
  };

  struct platform_driver mtk_driver = {
        .driver = {
                .name = "mtk-foo";
                .of_match_table = mtk_match_table,
                .of_probes = &mtk_probes;
        },
  };

And the probe functions might need to container_of() the device pointer to get the struct they know they need. The probe function could also be added to of_device_id and then we would have to look and see if that pointer is populated when the device is matched in generic device code.

Quoted 4 lines
Quoted 1 line

I guess I'll go down the path of extending the of_device_id structure?

Unfortunately, I don't think you can change of_device_id as it's part of the kernel ABI.

Ok. Then I'll go down the path of making it a parallel array?

maintainer
#10 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

In reply to Stephen Boyd
Quoted 40 lines · Stephen Boyd

We have a handful of clk drivers that have a collection of slightly variant device support keyed off of the compatible string. In each of these drivers, we demux the variant and then call the "real" probe function based on whatever is stored in the match data for that compatible string. Let's generalize this function so that it can be re-used as the platform_driver probe function directly.

Cc Ryder Lee <[email protected]>
Cc Rob Herring <[email protected]>
Cc Frank Rowand <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/of/device.c       | 31 +++++++++++++++++++++++++++++++
 include/linux/of_device.h |  1 +
 2 files changed, 32 insertions(+)
drivers/of/device.c +16 -0
index 0f27fad9fe94..8381f33ed2d8 100644
@@ -195,6 +195,37 @@ const void *of_device_get_match_data(const struct device *dev)
 }
 EXPORT_SYMBOL(of_device_get_match_data);
 
+/**
+ * platform_driver_probe_by_of_match_data - Probe a platform device using match data
+ * @pdev: platform device to probe
+ *
+ * For use by device drivers that multiplex their probe function through DT
+ * match data. Drivers can use this function to call their platform
+ * device probe directly without having to implement a wrapper function.
+ *
+ * static const struct of_device_id probe_funcs[] = {
+ *      { .compatible = "compat,foo", .data = foo_probe },
+ *      {}
+ * };
+ *
+ * struct platform_driver foo_driver = {
+ *      .probe = platform_driver_probe_by_of_match_data,
+ *      .driver = {

drivers/of/device.c const void *of_device_get_match_data(const struct device *dev) · line 223

Quoted 10 lines
drivers/of/device.c +10 -0
+ *            of_match_table = probe_funcs,
+ *      },
+ * };
+ *
+ */
+int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
+{
+	int (*probe_func)(struct platform_device *pdev);
+
+	probe_func = of_device_get_match_data(&pdev->dev);

Shouldn't we check if probe_func is not NULL?

Quoted 21 lines
drivers/of/device.c +5 -0
+
+	return probe_func(pdev);
+}
+EXPORT_SYMBOL_GPL(platform_driver_probe_by_of_match_data);
+
 static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
 {
 	const char *compat;
include/linux/of_device.h +1 -0
index 8d31e39dd564..4de84691d1c6 100644
@@ -33,6 +33,7 @@ extern int of_device_add(struct platform_device *pdev);
 extern int of_device_register(struct platform_device *ofdev);
 extern void of_device_unregister(struct platform_device *ofdev);
 
+extern int platform_driver_probe_by_of_match_data(struct platform_device *pdev);
 extern const void *of_device_get_match_data(const struct device *dev);
 
 extern ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len);
author maintainer
#11 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoting Matthias Brugger (2018-11-08 00:29:46)

Quoted 8 lines · Stephen Boyd

drivers/of/device.c const void *of_device_get_match_data(const struct device *dev) · line 223

Quoted 5 lines
drivers/of/device.c +5 -0
+int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
+{
+     int (*probe_func)(struct platform_device *pdev);
+
+     probe_func = of_device_get_match_data(&pdev->dev);

Shouldn't we check if probe_func is not NULL?

Is the oops from the NULL pointer deref insufficient?

maintainer
#12 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoted 10 lines · Stephen Boyd

Quoting Matthias Brugger (2018-11-08 00:29:46)

Quoted 8 lines · Stephen Boyd

drivers/of/device.c const void *of_device_get_match_data(const struct device *dev) · line 223

Quoted 5 lines
drivers/of/device.c +5 -0
+int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
+{
+     int (*probe_func)(struct platform_device *pdev);
+
+     probe_func = of_device_get_match_data(&pdev->dev);

Shouldn't we check if probe_func is not NULL?

Quoted 2 lines

Is the oops from the NULL pointer deref insufficient?

Do you think we should crash the machine if someone uses the call wrongly? Or should we provide the possibility to error out on the caller side?

Regards,
Matthias

#13 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Hi Matthias,

Quoted 11 lines · Matthias Brugger
Quoted 10 lines · Stephen Boyd

Quoting Matthias Brugger (2018-11-08 00:29:46)

Quoted 8 lines · Stephen Boyd

drivers/of/device.c const void *of_device_get_match_data(const struct device *dev) · line 223

Quoted 5 lines
drivers/of/device.c +5 -0
+int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
+{
+     int (*probe_func)(struct platform_device *pdev);
+
+     probe_func = of_device_get_match_data(&pdev->dev);

Shouldn't we check if probe_func is not NULL?

Quoted 4 lines
Quoted 1 line

Is the oops from the NULL pointer deref insufficient?

Do you think we should crash the machine if someone uses the call wrongly? Or should we provide the possibility to error out on the caller side?

I believe that would be a bug in the driver, to be discovered ASAP. So yes, please do crash ;-)

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
#14 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

Quoted 15 lines · Geert Uytterhoeven

Hi Matthias,

Quoted 11 lines · Matthias Brugger
Quoted 10 lines · Stephen Boyd

Quoting Matthias Brugger (2018-11-08 00:29:46)

Quoted 8 lines · Stephen Boyd

drivers/of/device.c const void *of_device_get_match_data(const struct device *dev) · line 223

Quoted 5 lines
drivers/of/device.c +5 -0
+int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
+{
+     int (*probe_func)(struct platform_device *pdev);
+
+     probe_func = of_device_get_match_data(&pdev->dev);

Shouldn't we check if probe_func is not NULL?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Is the oops from the NULL pointer deref insufficient?

Do you think we should crash the machine if someone uses the call wrongly? Or should we provide the possibility to error out on the caller side?

I believe that would be a bug in the driver, to be discovered ASAP. So yes, please do crash ;-)

Depends on the driver. If it's not needed to get a console out, then it should just WARN. Otherwise, you've got to go enable earlycon to see the crash. Of course, someone could just as easily stick data in here instead of a function ptr and we'll be back to a crash.

Rob

#15 lore raw

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

In reply to Geert Uytterhoeven
Quoted 14 lines · Geert Uytterhoeven

Hi Matthias,

Quoted 11 lines · Matthias Brugger
Quoted 10 lines · Stephen Boyd

Quoting Matthias Brugger (2018-11-08 00:29:46)

Quoted 8 lines · Stephen Boyd

drivers/of/device.c const void *of_device_get_match_data(const struct device *dev) · line 223

Quoted 5 lines
drivers/of/device.c +5 -0
+int platform_driver_probe_by_of_match_data(struct platform_device *pdev)
+{
+     int (*probe_func)(struct platform_device *pdev);
+
+     probe_func = of_device_get_match_data(&pdev->dev);

Shouldn't we check if probe_func is not NULL?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Is the oops from the NULL pointer deref insufficient?

Do you think we should crash the machine if someone uses the call wrongly? Or should we provide the possibility to error out on the caller side?

I believe that would be a bug in the driver, to be discovered ASAP. So yes, please do crash ;-)

This is one of Linus' pet peeves. He does not think crashing the machine is the proper choice (as a general statement).

-Frank

Quoted 5 lines

Gr{oetje,eeting}s,

Geert

author maintainer
#16 lore raw

PATCH 2/4 clk: mediatek: Convert to platform_driver_probe_by_match_data()

In reply to Stephen Boyd

Use this function to reduce the boiler-plate code that the mediatek clk driver needs to implement to probe clks for different devices within the same driver.

Cc Ryder Lee <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/clk/mediatek/clk-mt2701.c     | 20 +-------------------
 drivers/clk/mediatek/clk-mt2712.c     | 20 +-------------------
 drivers/clk/mediatek/clk-mt6797.c     | 20 +-------------------
 drivers/clk/mediatek/clk-mt7622-eth.c | 20 +-------------------
 drivers/clk/mediatek/clk-mt7622-hif.c | 20 +-------------------
 drivers/clk/mediatek/clk-mt7622.c     | 20 +-------------------
 6 files changed, 6 insertions(+), 114 deletions(-)
drivers/clk/mediatek/clk-mt2701.c +1 -19
index ab6ab07f53e6..5dd8fb98ce49 100644
@@ -1009,26 +1009,8 @@ static const struct of_device_id of_match_clk_mt2701[] = {
 	}
 };
 
-static int clk_mt2701_probe(struct platform_device *pdev)
-{
-	int (*clk_init)(struct platform_device *);
-	int r;
-
-	clk_init = of_device_get_match_data(&pdev->dev);
-	if (!clk_init)
-		return -EINVAL;
-
-	r = clk_init(pdev);
-	if (r)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt2701_drv = {
-	.probe = clk_mt2701_probe,
+	.probe = platform_driver_probe_by_of_match_data,
 	.driver = {
 		.name = "clk-mt2701",
 		.of_match_table = of_match_clk_mt2701,
drivers/clk/mediatek/clk-mt2712.c +1 -19
index 991d4093726e..17c031d110ff 100644
@@ -1441,26 +1441,8 @@ static const struct of_device_id of_match_clk_mt2712[] = {
 	}
 };
 
-static int clk_mt2712_probe(struct platform_device *pdev)
-{
-	int (*clk_probe)(struct platform_device *);
-	int r;
-
-	clk_probe = of_device_get_match_data(&pdev->dev);
-	if (!clk_probe)
-		return -EINVAL;
-
-	r = clk_probe(pdev);
-	if (r != 0)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt2712_drv = {
-	.probe = clk_mt2712_probe,
+	.probe = platform_driver_probe_by_of_match_data,
 	.driver = {
 		.name = "clk-mt2712",
 		.owner = THIS_MODULE,
drivers/clk/mediatek/clk-mt6797.c +1 -19
index 5702bc974ed9..2d7ff3098cc9 100644
@@ -680,26 +680,8 @@ static const struct of_device_id of_match_clk_mt6797[] = {
 	}
 };
 
-static int clk_mt6797_probe(struct platform_device *pdev)
-{
-	int (*clk_init)(struct platform_device *);
-	int r;
-
-	clk_init = of_device_get_match_data(&pdev->dev);
-	if (!clk_init)
-		return -EINVAL;
-
-	r = clk_init(pdev);
-	if (r)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt6797_drv = {
-	.probe = clk_mt6797_probe,
+	.probe = platform_driver_probe_by_of_match_data,
 	.driver = {
 		.name = "clk-mt6797",
 		.of_match_table = of_match_clk_mt6797,
drivers/clk/mediatek/clk-mt7622-eth.c +1 -19
index 6328127bbb3c..2ce825f19275 100644
@@ -127,26 +127,8 @@ static const struct of_device_id of_match_clk_mt7622_eth[] = {
 	}
 };
 
-static int clk_mt7622_eth_probe(struct platform_device *pdev)
-{
-	int (*clk_init)(struct platform_device *);
-	int r;
-
-	clk_init = of_device_get_match_data(&pdev->dev);
-	if (!clk_init)
-		return -EINVAL;
-
-	r = clk_init(pdev);
-	if (r)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt7622_eth_drv = {
-	.probe = clk_mt7622_eth_probe,
+	.probe = platform_driver_probe_by_of_match_data,
 	.driver = {
 		.name = "clk-mt7622-eth",
 		.of_match_table = of_match_clk_mt7622_eth,
drivers/clk/mediatek/clk-mt7622-hif.c +1 -19
index a6e8534276c6..65f13808155d 100644
@@ -140,26 +140,8 @@ static const struct of_device_id of_match_clk_mt7622_hif[] = {
 	}
 };
 
-static int clk_mt7622_hif_probe(struct platform_device *pdev)
-{
-	int (*clk_init)(struct platform_device *);
-	int r;
-
-	clk_init = of_device_get_match_data(&pdev->dev);
-	if (!clk_init)
-		return -EINVAL;
-
-	r = clk_init(pdev);
-	if (r)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt7622_hif_drv = {
-	.probe = clk_mt7622_hif_probe,
+	.probe = platform_driver_probe_by_of_match_data,
 	.driver = {
 		.name = "clk-mt7622-hif",
 		.of_match_table = of_match_clk_mt7622_hif,
drivers/clk/mediatek/clk-mt7622.c +1 -19
index 92f7e32770c6..d7310fb5964f 100644
@@ -746,26 +746,8 @@ static const struct of_device_id of_match_clk_mt7622[] = {
 	}
 };
 
-static int clk_mt7622_probe(struct platform_device *pdev)
-{
-	int (*clk_init)(struct platform_device *);
-	int r;
-
-	clk_init = of_device_get_match_data(&pdev->dev);
-	if (!clk_init)
-		return -EINVAL;
-
-	r = clk_init(pdev);
-	if (r)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt7622_drv = {
-	.probe = clk_mt7622_probe,
+	.probe = platform_driver_probe_by_of_match_data,
 	.driver = {
 		.name = "clk-mt7622",
 		.of_match_table = of_match_clk_mt7622,
Signature · 1 line
Sent by a computer through tubes
author maintainer
#17 lore raw

PATCH 3/4 clk: mediatek: Drop THIS_MODULE from platform_driver

In reply to Stephen Boyd

This is already set by platform_driver_register(), so we can remove it here.

Cc Ryder Lee <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/clk/mediatek/clk-mt2712.c | 1 -
 1 file changed, 1 deletion(-)
drivers/clk/mediatek/clk-mt2712.c +0 -1
index 17c031d110ff..55f81f9bfdb5 100644
@@ -1445,7 +1445,6 @@ static struct platform_driver clk_mt2712_drv = {
 	.probe = platform_driver_probe_by_of_match_data,
 	.driver = {
 		.name = "clk-mt2712",
-		.owner = THIS_MODULE,
 		.of_match_table = of_match_clk_mt2712,
 	},
 };
Signature · 1 line
Sent by a computer through tubes
author maintainer
#18 lore raw

PATCH 4/4 clk: mediatek: Simplify single driver probes

In reply to Stephen Boyd

We don't need to do the multiplex probe design here when we only have one compatible string. Just setup probe to point at the one probe function for now.

Cc Ryder Lee <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/clk/mediatek/clk-mt2701-g3d.c | 28 +++------------------------
 drivers/clk/mediatek/clk-mt7622-aud.c | 28 +++------------------------
 2 files changed, 6 insertions(+), 50 deletions(-)
drivers/clk/mediatek/clk-mt2701-g3d.c +3 -25
index 1328c112a38f..58b13f12051a 100644
@@ -58,34 +58,12 @@ static int clk_mt2701_g3dsys_init(struct platform_device *pdev)
 }
 
 static const struct of_device_id of_match_clk_mt2701_g3d[] = {
-	{
-		.compatible = "mediatek,mt2701-g3dsys",
-		.data = clk_mt2701_g3dsys_init,
-	}, {
-		/* sentinel */
-	}
+	{ .compatible = "mediatek,mt2701-g3dsys", },
+	{ /* sentinel */ }
 };
 
-static int clk_mt2701_g3d_probe(struct platform_device *pdev)
-{
-	int (*clk_init)(struct platform_device *);
-	int r;
-
-	clk_init = of_device_get_match_data(&pdev->dev);
-	if (!clk_init)
-		return -EINVAL;
-
-	r = clk_init(pdev);
-	if (r)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt2701_g3d_drv = {
-	.probe = clk_mt2701_g3d_probe,
+	.probe = clk_mt2701_g3dsys_init,
 	.driver = {
 		.name = "clk-mt2701-g3d",
 		.of_match_table = of_match_clk_mt2701_g3d,
drivers/clk/mediatek/clk-mt7622-aud.c +3 -25
index 4f3d47b41b3e..2b10d13a8a37 100644
@@ -171,34 +171,12 @@ static int clk_mt7622_audiosys_init(struct platform_device *pdev)
 }
 
 static const struct of_device_id of_match_clk_mt7622_aud[] = {
-	{
-		.compatible = "mediatek,mt7622-audsys",
-		.data = clk_mt7622_audiosys_init,
-	}, {
-		/* sentinel */
-	}
+	{ .compatible = "mediatek,mt7622-audsys", },
+	{ /* sentinel */ }
 };
 
-static int clk_mt7622_aud_probe(struct platform_device *pdev)
-{
-	int (*clk_init)(struct platform_device *);
-	int r;
-
-	clk_init = of_device_get_match_data(&pdev->dev);
-	if (!clk_init)
-		return -EINVAL;
-
-	r = clk_init(pdev);
-	if (r)
-		dev_err(&pdev->dev,
-			"could not register clock provider: %s: %d\n",
-			pdev->name, r);
-
-	return r;
-}
-
 static struct platform_driver clk_mt7622_aud_drv = {
-	.probe = clk_mt7622_aud_probe,
+	.probe = clk_mt7622_audiosys_init,
 	.driver = {
 		.name = "clk-mt7622-aud",
 		.of_match_table = of_match_clk_mt7622_aud,
Signature · 1 line
Sent by a computer through tubes