Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 5/6] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function
From: Krzysztof Kozlowski @ 2024-04-04  8:04 UTC (permalink / raw)
  To: Anand Moon
  Cc: Thinh Nguyen, Greg Kroah-Hartman, Alim Akhtar, Christophe JAILLET,
	Johan Hovold, linux-usb, linux-arm-kernel, linux-samsung-soc,
	linux-kernel
In-Reply-To: <CANAwSgSe37Pn062enCLd39P+RXU1VCZugyz7oDRafpnU1FyN1w@mail.gmail.com>

On 04/04/2024 09:38, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Thu, 4 Apr 2024 at 12:53, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 04/04/2024 09:13, Anand Moon wrote:
>>> Use devm_regulator_bulk_get_enable() instead of open coded
>>> 'devm_regulator_get(), regulator_enable(), regulator_disable().
>>
>> I fail to see how did you replace open-coded suspend/resume paths.
>>
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> ---
>>> V2: no changes, did not find any regression in pm suspend/resume.
>>
>> No, that's not equivalent code. No explanation in commit msg.
>>
>> You already got comments on this and nothing improved. You just entirely
>> ignored received comments. That's not how it works.
>>
>> I don't think you understand the code and Linux driver model. This patch
>> repeats several previous attempts with similar issues: no logic behind a
>> change.
>>
>> NAK.
> 
> devm_regulator_get_enable and devm_regulator_bulk_get_enable
> both remove the dependency from the driver to handle the regulator_enabled
> and regulator_disabled. ie this removes the regulator from the driver structure.

Not true. Please do not paste some generic knowledge and assume reviewer
knows it. Instead provide proof.

> 
> Since these functions set devm_add_action to disable the regulator when the
> resource is not used.
> 
>      ret = devm_add_action(dev, devm_regulator_bulk_disable, devres);
>      if (!ret)
>                return 0;

Listen, you already got comments on this at v1. Address previous
comments instead of repeating something unrelated. We should not have
the same discussion twice.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 08/23] media: i2c: imx258: Add support for 24MHz clock
From: Sakari Ailus @ 2024-04-04  8:03 UTC (permalink / raw)
  To: Luigi311
  Cc: git, linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <11f2e7d8-fd4c-4e14-81d8-cbc2cd2442fa@luigi311.com>

Hi Luigi311,

On Thu, Mar 28, 2024 at 11:55:24AM -0600, Luigi311 wrote:
> On 3/28/24 02:09, Sakari Ailus wrote:
> > Hi Luigi311,
> > 
> > Thank you for the patchset.
> > 
> > On Wed, Mar 27, 2024 at 05:16:54PM -0600, git@luigi311.com wrote:
> >> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> >>
> >> There's no reason why only a clock of 19.2MHz is supported.
> >> Indeed this isn't even a frequency listed in the datasheet.
> >>
> >> Add support for 24MHz as well.
> >> The PLL settings result in slightly different link frequencies,
> >> so parameterise those.
> >>
> >> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> >> Signed-off-by: Luigi311 <git@luigi311.com>
> > 
> > Is Luigi311 your real name? As per
> > Documentation/process/submitting-patches.rst, anonymous (or pseudonym I'd
> > say as well) contributions are not an option.
> 
> Luigi311 is not my real name but it would be a lot easier to find me if
> it was. My real name is Luis Garcia which is a super common name so its
> actually way easier to find me and all my work using my online name of
> Luigi311. I can go ahead and swap over to Luis Garcia if required but a
> name like that would provide no value in contacting/finding me since I'm
> not famous like all the other Luis Garcia's that appear on google.

Thanks. E-mail addresses are still unique, presumably, so that helps.

> 
> > 
> >> ---
> >>  drivers/media/i2c/imx258.c | 133 +++++++++++++++++++++++++++++--------
> >>  1 file changed, 107 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> >> index 351add1bc5d5..6ee7de079454 100644
> >> --- a/drivers/media/i2c/imx258.c
> >> +++ b/drivers/media/i2c/imx258.c
> >> @@ -76,9 +76,6 @@
> >>  #define REG_CONFIG_MIRROR_FLIP		0x03
> >>  #define REG_CONFIG_FLIP_TEST_PATTERN	0x02
> >>  
> >> -/* Input clock frequency in Hz */
> >> -#define IMX258_INPUT_CLOCK_FREQ		19200000
> >> -
> >>  struct imx258_reg {
> >>  	u16 address;
> >>  	u8 val;
> >> @@ -115,7 +112,9 @@ struct imx258_mode {
> >>  };
> >>  
> >>  /* 4208x3120 needs 1267Mbps/lane, 4 lanes */
> >> -static const struct imx258_reg mipi_data_rate_1267mbps[] = {
> >> +static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
> >> +	{ 0x0136, 0x13 },
> >> +	{ 0x0137, 0x33 },
> >>  	{ 0x0301, 0x05 },
> >>  	{ 0x0303, 0x02 },
> >>  	{ 0x0305, 0x03 },
> >> @@ -133,7 +132,29 @@ static const struct imx258_reg mipi_data_rate_1267mbps[] = {
> >>  	{ 0x0823, 0xCC },
> >>  };
> >>  
> >> -static const struct imx258_reg mipi_data_rate_640mbps[] = {
> >> +static const struct imx258_reg mipi_1272mbps_24mhz[] = {
> >> +	{ 0x0136, 0x18 },
> >> +	{ 0x0137, 0x00 },
> >> +	{ 0x0301, 0x05 },
> >> +	{ 0x0303, 0x02 },
> >> +	{ 0x0305, 0x04 },
> >> +	{ 0x0306, 0x00 },
> >> +	{ 0x0307, 0xD4 },
> >> +	{ 0x0309, 0x0A },
> >> +	{ 0x030B, 0x01 },
> >> +	{ 0x030D, 0x02 },
> >> +	{ 0x030E, 0x00 },
> >> +	{ 0x030F, 0xD8 },
> >> +	{ 0x0310, 0x00 },
> >> +	{ 0x0820, 0x13 },
> >> +	{ 0x0821, 0x4C },
> >> +	{ 0x0822, 0xCC },
> >> +	{ 0x0823, 0xCC },
> >> +};
> >> +
> >> +static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
> >> +	{ 0x0136, 0x13 },
> >> +	{ 0x0137, 0x33 },
> >>  	{ 0x0301, 0x05 },
> >>  	{ 0x0303, 0x02 },
> >>  	{ 0x0305, 0x03 },
> >> @@ -151,9 +172,27 @@ static const struct imx258_reg mipi_data_rate_640mbps[] = {
> >>  	{ 0x0823, 0x00 },
> >>  };
> >>  
> >> +static const struct imx258_reg mipi_642mbps_24mhz[] = {
> >> +	{ 0x0136, 0x18 },
> >> +	{ 0x0137, 0x00 },
> >> +	{ 0x0301, 0x05 },
> >> +	{ 0x0303, 0x02 },
> >> +	{ 0x0305, 0x04 },
> >> +	{ 0x0306, 0x00 },
> >> +	{ 0x0307, 0x6B },
> >> +	{ 0x0309, 0x0A },
> >> +	{ 0x030B, 0x01 },
> >> +	{ 0x030D, 0x02 },
> >> +	{ 0x030E, 0x00 },
> >> +	{ 0x030F, 0xD8 },
> >> +	{ 0x0310, 0x00 },
> >> +	{ 0x0820, 0x0A },
> >> +	{ 0x0821, 0x00 },
> >> +	{ 0x0822, 0x00 },
> >> +	{ 0x0823, 0x00 },
> >> +};
> >> +
> >>  static const struct imx258_reg mode_common_regs[] = {
> >> -	{ 0x0136, 0x13 },
> >> -	{ 0x0137, 0x33 },
> >>  	{ 0x3051, 0x00 },
> >>  	{ 0x3052, 0x00 },
> >>  	{ 0x4E21, 0x14 },
> >> @@ -313,10 +352,6 @@ static const char * const imx258_supply_name[] = {
> >>  
> >>  #define IMX258_NUM_SUPPLIES ARRAY_SIZE(imx258_supply_name)
> >>  
> >> -/* Configurations for supported link frequencies */
> >> -#define IMX258_LINK_FREQ_634MHZ	633600000ULL
> >> -#define IMX258_LINK_FREQ_320MHZ	320000000ULL
> >> -
> >>  enum {
> >>  	IMX258_LINK_FREQ_1267MBPS,
> >>  	IMX258_LINK_FREQ_640MBPS,
> >> @@ -335,25 +370,55 @@ static u64 link_freq_to_pixel_rate(u64 f)
> >>  }
> >>  
> >>  /* Menu items for LINK_FREQ V4L2 control */
> >> -static const s64 link_freq_menu_items[] = {
> >> +/* Configurations for supported link frequencies */
> >> +#define IMX258_LINK_FREQ_634MHZ	633600000ULL
> >> +#define IMX258_LINK_FREQ_320MHZ	320000000ULL
> >> +
> >> +static const s64 link_freq_menu_items_19_2[] = {
> >>  	IMX258_LINK_FREQ_634MHZ,
> >>  	IMX258_LINK_FREQ_320MHZ,
> >>  };
> >>  
> >> +/* Configurations for supported link frequencies */
> >> +#define IMX258_LINK_FREQ_636MHZ	636000000ULL
> >> +#define IMX258_LINK_FREQ_321MHZ	321000000ULL
> > 
> > These values aren't used outside the array below and the macro names are
> > imprecise anyway. Could you put the numerical values to the array instead?
> 
> Ok I've removed the defines and just threw the values into the array instead.
> 
> > 
> >> +
> >> +static const s64 link_freq_menu_items_24[] = {
> >> +	IMX258_LINK_FREQ_636MHZ,
> >> +	IMX258_LINK_FREQ_321MHZ,
> >> +};
> >> +
> >>  /* Link frequency configs */
> >> -static const struct imx258_link_freq_config link_freq_configs[] = {
> >> +static const struct imx258_link_freq_config link_freq_configs_19_2[] = {
> >>  	[IMX258_LINK_FREQ_1267MBPS] = {
> >>  		.pixels_per_line = IMX258_PPL_DEFAULT,
> >>  		.reg_list = {
> >> -			.num_of_regs = ARRAY_SIZE(mipi_data_rate_1267mbps),
> >> -			.regs = mipi_data_rate_1267mbps,
> >> +			.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz),
> >> +			.regs = mipi_1267mbps_19_2mhz,
> >>  		}
> >>  	},
> >>  	[IMX258_LINK_FREQ_640MBPS] = {
> >>  		.pixels_per_line = IMX258_PPL_DEFAULT,
> >>  		.reg_list = {
> >> -			.num_of_regs = ARRAY_SIZE(mipi_data_rate_640mbps),
> >> -			.regs = mipi_data_rate_640mbps,
> >> +			.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz),
> >> +			.regs = mipi_640mbps_19_2mhz,
> >> +		}
> >> +	},
> >> +};
> >> +
> >> +static const struct imx258_link_freq_config link_freq_configs_24[] = {
> >> +	[IMX258_LINK_FREQ_1267MBPS] = {
> >> +		.pixels_per_line = IMX258_PPL_DEFAULT,
> >> +		.reg_list = {
> >> +			.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz),
> >> +			.regs = mipi_1272mbps_24mhz,
> >> +		}
> >> +	},
> >> +	[IMX258_LINK_FREQ_640MBPS] = {
> >> +		.pixels_per_line = IMX258_PPL_DEFAULT,
> >> +		.reg_list = {
> >> +			.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz),
> >> +			.regs = mipi_642mbps_24mhz,
> >>  		}
> >>  	},
> >>  };
> >> @@ -410,6 +475,9 @@ struct imx258 {
> >>  	/* Current mode */
> >>  	const struct imx258_mode *cur_mode;
> >>  
> >> +	const struct imx258_link_freq_config *link_freq_configs;
> >> +	const s64 *link_freq_menu_items;
> >> +
> >>  	/*
> >>  	 * Mutex for serialized access:
> >>  	 * Protect sensor module set pad format and start/stop streaming safely.
> >> @@ -713,7 +781,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
> >>  		imx258->cur_mode = mode;
> >>  		__v4l2_ctrl_s_ctrl(imx258->link_freq, mode->link_freq_index);
> >>  
> >> -		link_freq = link_freq_menu_items[mode->link_freq_index];
> >> +		link_freq = imx258->link_freq_menu_items[mode->link_freq_index];
> >>  		pixel_rate = link_freq_to_pixel_rate(link_freq);
> >>  		__v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate, pixel_rate);
> >>  		/* Update limits and set FPS to default */
> >> @@ -727,7 +795,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
> >>  			vblank_def);
> >>  		__v4l2_ctrl_s_ctrl(imx258->vblank, vblank_def);
> >>  		h_blank =
> >> -			link_freq_configs[mode->link_freq_index].pixels_per_line
> >> +			imx258->link_freq_configs[mode->link_freq_index].pixels_per_line
> >>  			 - imx258->cur_mode->width;
> >>  		__v4l2_ctrl_modify_range(imx258->hblank, h_blank,
> >>  					 h_blank, 1, h_blank);
> >> @@ -747,7 +815,7 @@ static int imx258_start_streaming(struct imx258 *imx258)
> >>  
> >>  	/* Setup PLL */
> >>  	link_freq_index = imx258->cur_mode->link_freq_index;
> >> -	reg_list = &link_freq_configs[link_freq_index].reg_list;
> >> +	reg_list = &imx258->link_freq_configs[link_freq_index].reg_list;
> >>  	ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs);
> >>  	if (ret) {
> >>  		dev_err(&client->dev, "%s failed to set plls\n", __func__);
> >> @@ -946,9 +1014,9 @@ static int imx258_init_controls(struct imx258 *imx258)
> >>  	imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> >>  				&imx258_ctrl_ops,
> >>  				V4L2_CID_LINK_FREQ,
> >> -				ARRAY_SIZE(link_freq_menu_items) - 1,
> >> +				ARRAY_SIZE(link_freq_menu_items_19_2) - 1,
> >>  				0,
> >> -				link_freq_menu_items);
> >> +				imx258->link_freq_menu_items);
> >>  
> >>  	if (imx258->link_freq)
> >>  		imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> >> @@ -964,8 +1032,10 @@ static int imx258_init_controls(struct imx258 *imx258)
> >>  	if (vflip)
> >>  		vflip->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> >>  
> >> -	pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
> >> -	pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
> >> +	pixel_rate_max =
> >> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[0]);
> >> +	pixel_rate_min =
> >> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[1]);
> > 
> > The arrays currently have two entries so this works but it'd nice to have a
> > bit more robust way to handle differences between the two arrays. Could you
> > maintain e.g. the number of entries in the array in a struct field perhaps?
> 
> Would it make more sense to do something like default to index 0 and then use 
> ARRAY_SIZE to iterate through the array and do a comparison to get the min and
> max size so it would always choose the correct value no matter how many entries
> there are?

I'll check that later patch you mentioned in the follow-up separately.

-- 
Kind regards,

Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 2/4] dt-bindings: arm: fsl: remove reduntant toradex,colibri-imx8x
From: Francesco Dolcini @ 2024-04-04  8:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Hiago De Franco, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Krzysztof Kozlowski, Conor Dooley, Hiago De Franco,
	imx, linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20240403165721.GA3980362-robh@kernel.org>

Hello Rob,

On Wed, Apr 03, 2024 at 11:57:21AM -0500, Rob Herring wrote:
> On Tue, Apr 02, 2024 at 04:35:10PM -0300, Hiago De Franco wrote:
> > From: Hiago De Franco <hiago.franco@toradex.com>
> > 
> > 'toradex,colibri-imx8x' is already present as a constant value for
> > 'i.MX8QP Board with Toradex Colibri iMX8X Modules', so there is no need
> > to keep it twice as a enum value for 'i.MX8QXP based Boards'.
> 
> If the module can operate on its own

For the records, it can't.

> Acked-by: Rob Herring <robh@kernel.org>

Thanks,
Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH net-next 2/3] net: sparx5: add support for tc flower mirred action.
From: Daniel Machon @ 2024-04-04  7:58 UTC (permalink / raw)
  To: Horatiu Vultur
  Cc: Lars Povlsen, Steen Hegelund, UNGLinuxDriver, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-arm-kernel,
	netdev, linux-kernel
In-Reply-To: <20240404064627.6m25m2mi7zbpqbyo@DEN-DL-M31836.microchip.com>

> The 04/03/2024 20:41, Daniel Machon wrote:
> 
> Hi Daniel,
> 
> ...
> 
> > +static int sparx5_tc_action_mirred(struct vcap_admin *admin,
> > +				   struct vcap_rule *vrule,
> > +				   struct flow_cls_offload *fco,
> > +				   struct flow_action_entry *act)
> > +{
> > +	struct vcap_u72_action ports = {0};
> 
> Maybe this is just a preferences, but usually we use memset instead of {0};

Yes, I think this falls under preference. I'd like to keep this one as
is.

> 
> > +	int err;
> > +
> > +	if (admin->vtype != VCAP_TYPE_IS0 && admin->vtype != VCAP_TYPE_IS2) {
> > +		NL_SET_ERR_MSG_MOD(fco->common.extack,
> > +				   "Mirror action not supported in this VCAP");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	err = vcap_rule_add_action_u32(vrule, VCAP_AF_MASK_MODE,
> > +				       SPX5_PMM_OR_DSTMASK);
> > +	if (err)
> > +		return err;
> > +
> > +	sparx5_tc_flower_set_port_mask(&ports, act->dev);
> > +
> > +	err = vcap_rule_add_action_u72(vrule, VCAP_AF_PORT_MASK, &ports);
> > +	if (err)
> > +		return err;
> 
> You can just return directly the return value from vcap_rule_add_action_u72
> Something like:
> 
> return vcap_rule_add_action_u72(...)
>

Yes, seems like a reasonable change :-) I need to respin anyway, since
NIPA is complaining about something I didn't catch in my local run. Will
incorporate changes in v2 - thanks.

> > +
> > +	return 0;
> > +}
> > -- 
> > 2.34.1
> > 
> 
> -- 
> /Horatiu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v2 3/3] drm/mediatek: drm_ddp_comp: Add mtk_ddp_is_simple_comp() internal helper
From: AngeloGioacchino Del Regno @ 2024-04-04  7:57 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: p.zabel, airlied, daniel, matthias.bgg, angelogioacchino.delregno,
	ck.hu, nancy.lin, nathan.lu, dri-devel, linux-mediatek,
	linux-kernel, linux-arm-kernel, kernel, wenst
In-Reply-To: <20240404075754.83455-1-angelogioacchino.delregno@collabora.com>

Move the simple component check to a new mtk_ddp_is_simple_comp()
internal helper to reduce code duplication.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 57 +++++++++++----------
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 477fc1950a0e..d760285761b9 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -576,6 +576,29 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
 	return ret;
 }
 
+static bool mtk_ddp_is_simple_comp(enum mtk_ddp_comp_type type)
+{
+	switch (type) {
+	case MTK_DISP_AAL:
+	case MTK_DISP_BLS:
+	case MTK_DISP_CCORR:
+	case MTK_DISP_COLOR:
+	case MTK_DISP_GAMMA:
+	case MTK_DISP_MERGE:
+	case MTK_DISP_OVL:
+	case MTK_DISP_OVL_2L:
+	case MTK_DISP_OVL_ADAPTOR:
+	case MTK_DISP_PWM:
+	case MTK_DISP_RDMA:
+	case MTK_DP_INTF:
+	case MTK_DPI:
+	case MTK_DSI:
+		return false;
+	default:
+		return true;
+	}
+}
+
 int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
 		      unsigned int comp_id)
 {
@@ -606,19 +629,13 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
 	}
 	comp->dev = &comp_pdev->dev;
 
-	if (type == MTK_DISP_AAL ||
-	    type == MTK_DISP_BLS ||
-	    type == MTK_DISP_CCORR ||
-	    type == MTK_DISP_COLOR ||
-	    type == MTK_DISP_GAMMA ||
-	    type == MTK_DISP_MERGE ||
-	    type == MTK_DISP_OVL ||
-	    type == MTK_DISP_OVL_2L ||
-	    type == MTK_DISP_PWM ||
-	    type == MTK_DISP_RDMA ||
-	    type == MTK_DPI ||
-	    type == MTK_DP_INTF ||
-	    type == MTK_DSI)
+	/*
+	 * Resources for simple components are retrieved here as those are
+	 * managed in here without the need of more complex drivers; for
+	 * the latter, their respective probe function will do the job, so
+	 * we must avoid getting their resources here.
+	 */
+	if (!mtk_ddp_is_simple_comp(type))
 		return 0;
 
 	priv = devm_kzalloc(comp->dev, sizeof(*priv), GFP_KERNEL);
@@ -652,19 +669,7 @@ void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp)
 		return;
 
 	/* Complex components are destroyed with their own remove callback */
-	if (mtk_ddp_matches[comp->id].type == MTK_DISP_AAL ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_BLS ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_CCORR ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_COLOR ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_GAMMA ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_MERGE ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_OVL ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_OVL_2L ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_PWM ||
-	    mtk_ddp_matches[comp->id].type == MTK_DISP_RDMA ||
-	    mtk_ddp_matches[comp->id].type == MTK_DPI ||
-	    mtk_ddp_matches[comp->id].type == MTK_DP_INTF ||
-	    mtk_ddp_matches[comp->id].type == MTK_DSI)
+	if (!mtk_ddp_is_simple_comp(mtk_ddp_matches[comp->id].type))
 		return;
 
 	priv = dev_get_drvdata(comp->dev);
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 1/3] drm/mediatek: drm_ddp_comp: Fix and cleanup DDP component CRTC search
From: AngeloGioacchino Del Regno @ 2024-04-04  7:57 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: p.zabel, airlied, daniel, matthias.bgg, angelogioacchino.delregno,
	ck.hu, nancy.lin, nathan.lu, dri-devel, linux-mediatek,
	linux-kernel, linux-arm-kernel, kernel, wenst
In-Reply-To: <20240404075754.83455-1-angelogioacchino.delregno@collabora.com>

Finding a possible CRTC by DDP component is done by first checking
static routes in three paths (main, external, third/extra path) and
then, if not found, we check for dynamic connection on a per-route
basis because, for example, on some SoCs the main route may output
to either a DSI display or DisplayPort and this is finally done by
assigning a CRTC mask to `possible_crtcs`, found with function
mtk_drm_find_comp_in_ddp_conn_path(): being that a mask the possible
values are BIT(x) and, if no CRTC is possible, zero.

Problem is, both mtk_drm_find_possible_crtc_by_comp() and the
aforementioned function are trying to return a negative error value
(but it's unsigned int, so the value is never negative!) if no CRTC
was found, which is wrong for multiple obvious reasons.

Cleanup both functions, so that:
 - mtk_drm_find_comp_in_ddp_conn_path() returns a signed integer
   with a negative number for error, or a bit/bitmask of the found
   possible CRTC; and
 - mtk_drm_find_possible_crtc_by_comp() always returns either a
   bitmask of the possible CRTC, or zero if none available.

Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 44 ++++++++++-----------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index a515e96cfefc..82d7a6887f44 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -514,29 +514,25 @@ static bool mtk_drm_find_comp_in_ddp(struct device *dev,
 	return false;
 }
 
-static unsigned int mtk_drm_find_comp_in_ddp_conn_path(struct device *dev,
-						       const struct mtk_drm_route *routes,
-						       unsigned int num_routes,
-						       struct mtk_ddp_comp *ddp_comp)
+static int mtk_drm_find_comp_in_ddp_conn_path(struct device *dev,
+					      const struct mtk_drm_route *routes,
+					      unsigned int num_routes,
+					      struct mtk_ddp_comp *ddp_comp)
 {
-	int ret;
-	unsigned int i;
+	int i;
 
-	if (!routes) {
-		ret = -EINVAL;
-		goto err;
+	if (!routes || !num_routes) {
+		DRM_ERROR("No connection routes specified!\n");
+		return -EINVAL;
 	}
 
 	for (i = 0; i < num_routes; i++)
 		if (dev == ddp_comp[routes[i].route_ddp].dev)
 			return BIT(routes[i].crtc_id);
 
-	ret = -ENODEV;
-err:
-
-	DRM_INFO("Failed to find comp in ddp table, ret = %d\n", ret);
+	DRM_ERROR("Failed to find component in ddp table\n");
 
-	return 0;
+	return -ENODEV;
 }
 
 int mtk_ddp_comp_get_id(struct device_node *node,
@@ -558,22 +554,24 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
 						struct device *dev)
 {
 	struct mtk_drm_private *private = drm->dev_private;
-	unsigned int ret = 0;
+	int ret;
 
 	if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path, private->data->main_len,
 				     private->ddp_comp))
-		ret = BIT(0);
+		return BIT(0);
 	else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
 					  private->data->ext_len, private->ddp_comp))
-		ret = BIT(1);
+		return BIT(1);
 	else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
 					  private->data->third_len, private->ddp_comp))
-		ret = BIT(2);
-	else
-		ret = mtk_drm_find_comp_in_ddp_conn_path(dev,
-							 private->data->conn_routes,
-							 private->data->num_conn_routes,
-							 private->ddp_comp);
+		return BIT(2);
+
+	ret = mtk_drm_find_comp_in_ddp_conn_path(dev, private->data->conn_routes,
+						 private->data->num_conn_routes,
+						 private->ddp_comp);
+	/* No CRTC is available: return a zero mask */
+	if (ret < 0)
+		return 0;
 
 	return ret;
 }
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 2/3] drm/mediatek: Perform iounmap on simple DDP component destruction
From: AngeloGioacchino Del Regno @ 2024-04-04  7:57 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: p.zabel, airlied, daniel, matthias.bgg, angelogioacchino.delregno,
	ck.hu, nancy.lin, nathan.lu, dri-devel, linux-mediatek,
	linux-kernel, linux-arm-kernel, kernel, wenst
In-Reply-To: <20240404075754.83455-1-angelogioacchino.delregno@collabora.com>

Add a new mtk_ddp_comp_destroy() function and call it in the teardown
path of mtk_drm_drv to make sure that we unmap the iospace of the
simple DDP components.

While at it, also fix iounmapping on mtk_ddp_comp_init() error path.

Fixes: ff1395609e20 ("drm/mediatek: Move mtk_ddp_comp_init() from sub driver to DRM driver")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 38 ++++++++++++++++++++-
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |  4 ++-
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 82d7a6887f44..477fc1950a0e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -627,8 +627,11 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
 
 	priv->regs = of_iomap(node, 0);
 	priv->clk = of_clk_get(node, 0);
-	if (IS_ERR(priv->clk))
+	if (IS_ERR(priv->clk)) {
+		iounmap(priv->regs);
+		priv->regs = NULL;
 		return PTR_ERR(priv->clk);
+	}
 
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
 	ret = cmdq_dev_get_client_reg(comp->dev, &priv->cmdq_reg, 0);
@@ -640,3 +643,36 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
 
 	return 0;
 }
+
+void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp)
+{
+	struct mtk_ddp_comp_dev *priv;
+
+	if (!comp || !comp->dev)
+		return;
+
+	/* Complex components are destroyed with their own remove callback */
+	if (mtk_ddp_matches[comp->id].type == MTK_DISP_AAL ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_BLS ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_CCORR ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_COLOR ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_GAMMA ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_MERGE ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_OVL ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_OVL_2L ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_PWM ||
+	    mtk_ddp_matches[comp->id].type == MTK_DISP_RDMA ||
+	    mtk_ddp_matches[comp->id].type == MTK_DPI ||
+	    mtk_ddp_matches[comp->id].type == MTK_DP_INTF ||
+	    mtk_ddp_matches[comp->id].type == MTK_DSI)
+		return;
+
+	priv = dev_get_drvdata(comp->dev);
+	if (!priv)
+		return;
+
+	if (priv->regs) {
+		iounmap(priv->regs);
+		priv->regs = NULL;
+	}
+}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 93d79a1366e9..3d147217caa4 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -330,6 +330,7 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
 						struct device *dev);
 int mtk_ddp_comp_init(struct device_node *comp_node, struct mtk_ddp_comp *comp,
 		      unsigned int comp_id);
+void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp);
 enum mtk_ddp_comp_type mtk_ddp_comp_get_type(unsigned int comp_id);
 void mtk_ddp_write(struct cmdq_pkt *cmdq_pkt, unsigned int value,
 		   struct cmdq_client_reg *cmdq_reg, void __iomem *regs,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 74832c213092..7e240d16d3e5 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -946,8 +946,10 @@ static void mtk_drm_remove(struct platform_device *pdev)
 	component_master_del(&pdev->dev, &mtk_drm_ops);
 	pm_runtime_disable(&pdev->dev);
 	of_node_put(private->mutex_node);
-	for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++)
+	for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++) {
+		mtk_ddp_comp_destroy(&private->ddp_comp[i]);
 		of_node_put(private->comp_node[i]);
+	}
 }
 
 static int mtk_drm_sys_prepare(struct device *dev)
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 0/3] drm/mediatek: Fixes for DDP component search/destroy
From: AngeloGioacchino Del Regno @ 2024-04-04  7:57 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: p.zabel, airlied, daniel, matthias.bgg, angelogioacchino.delregno,
	ck.hu, nancy.lin, nathan.lu, dri-devel, linux-mediatek,
	linux-kernel, linux-arm-kernel, kernel, wenst

Changes in v2:
 - Fixed patch [2/3]

This series performs some cleanups for DDP component CRTC search and
correctly iounmaps the previously of_iomap() calls from drm_ddp_comp.

Tested on MT8195 Cherry Tomato

AngeloGioacchino Del Regno (3):
  drm/mediatek: drm_ddp_comp: Fix and cleanup DDP component CRTC search
  drm/mediatek: Perform iounmap on simple DDP component destruction
  drm/mediatek: drm_ddp_comp: Add mtk_ddp_is_simple_comp() internal
    helper

 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 113 +++++++++++++-------
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   4 +-
 3 files changed, 80 insertions(+), 38 deletions(-)

-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 4/4] clk: en7523: add EN7581 support
From: Lorenzo Bianconi @ 2024-04-04  7:55 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-clk, mturquette, sboyd, linux-arm-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
	catalin.marinas, will, upstream, lorenzo.bianconi83,
	angelogioacchino.delregno
In-Reply-To: <95d194ed-86fe-45df-88f1-ae460eb36eaf@linaro.org>


[-- Attachment #1.1: Type: text/plain, Size: 1781 bytes --]

> On 03/04/2024 18:20, Lorenzo Bianconi wrote:
> > Introduce EN7581 clock support to clk-en7523 driver.
> > 
> > Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> 
> > +	return 0;
> > +}
> > +
> >  static int en7523_clk_probe(struct platform_device *pdev)
> >  {
> >  	struct device_node *node = pdev->dev.of_node;
> > @@ -306,6 +413,12 @@ static int en7523_clk_probe(struct platform_device *pdev)
> >  	if (IS_ERR(np_base))
> >  		return PTR_ERR(np_base);
> >  
> > +	if (of_device_is_compatible(node, "airoha,en7581-scu")) {
> 
> Having matching and compatible comparisons inside various code is
> discouraged. Does not scale. Use driver/match data to store some sort of
> flags and check for the flag or some other parameter. The best if
> compatible appears once and only once: in of_device_id.

ack, I will fix it.

Regards,
Lorenzo

> 
> > +		r = en7581_clk_hw_init(pdev, base, np_base);
> > +		if (r)
> > +			return r;
> > +	}
> > +
> >  	clk_data = devm_kzalloc(&pdev->dev,
> >  				struct_size(clk_data, hws, EN7523_NUM_CLOCKS),
> >  				GFP_KERNEL);
> > @@ -329,8 +442,15 @@ static const struct clk_ops en7523_pcie_ops = {
> >  	.unprepare = en7523_pci_unprepare,
> >  };
> >  
> > +static const struct clk_ops en7581_pcie_ops = {
> > +	.is_enabled = en7581_pci_is_enabled,
> > +	.prepare = en7581_pci_prepare,
> > +	.unprepare = en7581_pci_unprepare,
> > +};
> > +
> >  static const struct of_device_id of_match_clk_en7523[] = {
> >  	{ .compatible = "airoha,en7523-scu", .data = &en7523_pcie_ops },
> > +	{ .compatible = "airoha,en7581-scu", .data = &en7581_pcie_ops },
> >  	{ /* sentinel */ }
> >  };
> >  
> 
> Best regards,
> Krzysztof
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 3/6] usb: ohci-exynos: Use devm_clk_get_enabled() helpers
From: Anand Moon @ 2024-04-04  7:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Alan Stern, Greg Kroah-Hartman, Alim Akhtar, Christophe JAILLET,
	Johan Hovold, linux-usb, linux-arm-kernel, linux-samsung-soc,
	linux-kernel
In-Reply-To: <0a21f8e8-f4a6-4311-ab35-a02a1a593629@linaro.org>

Hi Krzysztof,


On Thu, 4 Apr 2024 at 12:50, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/04/2024 09:13, Anand Moon wrote:
> > The devm_clk_get_enabled() helpers:
> >     - call devm_clk_get()
> >     - call clk_prepare_enable() and register what is needed in order to
> >       call clk_disable_unprepare() when needed, as a managed resource.
> >
> > This simplifies the code and avoids the calls to clk_disable_unprepare().
> >
> > While at it, use dev_err_probe consistently, and use its return value
> > to return the error code.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > v2: new changes in this series.
> > ---
> >  drivers/usb/host/ohci-exynos.c | 19 +++++--------------
> >  1 file changed, 5 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > index 20e26a474591..85d04ae0ae40 100644
> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -135,20 +135,15 @@ static int exynos_ohci_probe(struct platform_device *pdev)
> >
> >       err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci);
> >       if (err)
> > -             goto fail_clk;
> > -
> > -     exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
> > +             goto fail_io;
> >
> > +     exynos_ohci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost");
> >       if (IS_ERR(exynos_ohci->clk)) {
> > -             dev_err(&pdev->dev, "Failed to get usbhost clock\n");
> > -             err = PTR_ERR(exynos_ohci->clk);
> > -             goto fail_clk;
> > +             usb_put_hcd(hcd);
> > +             return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ohci->clk),
> > +                             "Failed to get usbhost clock\n");
>
> Why do you introduce entirely parallel exit paths? There is already
> single error handling part with labels. Use that.
>
OK, I  will use the labels to return over here,
>
>
> Best regards,
> Krzysztof
>
Thanks

-Anand

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm64: dts: ti: k3-j722s-evm: Enable eMMC support
From: Francesco Dolcini @ 2024-04-04  7:45 UTC (permalink / raw)
  To: Michael Walle
  Cc: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
	linux-kernel
In-Reply-To: <20240403102302.3934932-1-mwalle@kernel.org>

Hello Michael,

On Wed, Apr 03, 2024 at 12:23:02PM +0200, Michael Walle wrote:
> The J722S EVM has an on-board eMMC. Enable the SDHC interface for it.
> There is no pinmuxing required because the interface has dedicated pins.
> 
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> v2:
>  - move status="okay" last
> ---
>  arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> index cee3a8661d5e..6b148da2bcdc 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> @@ -369,6 +369,13 @@ partition@3fc0000 {
>  
>  };
>  
> +&sdhci0 {
> +	ti,driver-strength-ohm = <50>;

According to the latest DTS coding style guidelines this needs to be
just before status.

https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node

Yes, we would need a checker, I know and I agree. At least now we moved
out of the tribal knowledge way of working with every maintainer having
slightly different expectation on this topic ... one step at a time :-)

> +	disable-wp;
> +	bootph-all;
> +	status = "okay";
> +};


Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 0/8] cpufreq: sun50i: Add Allwinner H616 support
From: Ryan Walklin @ 2024-04-04  7:44 UTC (permalink / raw)
  To: Viresh Kumar, Andre Przywara
  Cc: Yangtao Li, Viresh Kumar, Nishanth Menon, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Rafael J . Wysocki, linux-pm,
	devicetree, linux-sunxi, linux-arm-kernel, Brandon Cheo Fusi,
	Martin Botka, Martin Botka, Chris Morgan, Mark Rutland,
	Lorenzo Pieralisi, Sudeep Holla
In-Reply-To: <20240404064027.shjqvqih7s5rnv2i@vireshk-i7>

On Thu, 4 Apr 2024, at 7:40 PM, Viresh Kumar wrote:
> Is it okay to merge all the changes via the cpufreq tree ?

I have tested this series with an H700-based board, and have at least one speed-bin (1.032GHz) is not supported although the governor attempts to enable it based on the opp-supported-hw bitmask, and I am unable to reach the 1.5GHz bin at 1.16v (or higher) despite it working on the vendor BSP (kernel panic at boot if enabled), so this may need some slight rework.

I have reached out to Andre on IRC to debug.

Ryan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm64: dts: debix-a: Disable i2c2 in base .dts
From: Marco Felsch @ 2024-04-04  7:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: devicetree, imx, linux-arm-kernel, Rob Herring, Jacopo Mondi,
	Pengutronix Kernel Team, Shawn Guo, Sascha Hauer, Conor Dooley,
	Krzysztof Kozlowski, Jacopo Mondi, Fabio Estevam
In-Reply-To: <20240404002009.10052-1-laurent.pinchart@ideasonboard.com>

On 24-04-04, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>
> 
> The I2C2 bus is used for the CSI and DSI connectors only, no devices are
> connected to it on neither the Debix Model A nor its IO board. Disable
> the bus in the board's .dts and remove its clock frequency settings, as
> the value depends solely on the devices conncted to the CSI and DSI
> connectors. Display panel or camera sensor overlays will configure and
> enable the bus when necessary.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 5/6] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function
From: Anand Moon @ 2024-04-04  7:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Thinh Nguyen, Greg Kroah-Hartman, Alim Akhtar, Christophe JAILLET,
	Johan Hovold, linux-usb, linux-arm-kernel, linux-samsung-soc,
	linux-kernel
In-Reply-To: <d6d5f6d4-1d34-4d42-9afc-822a014063dd@linaro.org>

Hi Krzysztof,

On Thu, 4 Apr 2024 at 12:53, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/04/2024 09:13, Anand Moon wrote:
> > Use devm_regulator_bulk_get_enable() instead of open coded
> > 'devm_regulator_get(), regulator_enable(), regulator_disable().
>
> I fail to see how did you replace open-coded suspend/resume paths.
>
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > V2: no changes, did not find any regression in pm suspend/resume.
>
> No, that's not equivalent code. No explanation in commit msg.
>
> You already got comments on this and nothing improved. You just entirely
> ignored received comments. That's not how it works.
>
> I don't think you understand the code and Linux driver model. This patch
> repeats several previous attempts with similar issues: no logic behind a
> change.
>
> NAK.

devm_regulator_get_enable and devm_regulator_bulk_get_enable
both remove the dependency from the driver to handle the regulator_enabled
and regulator_disabled. ie this removes the regulator from the driver structure.

Since these functions set devm_add_action to disable the regulator when the
resource is not used.

     ret = devm_add_action(dev, devm_regulator_bulk_disable, devres);
     if (!ret)
               return 0;
>
> Best regards,
> Krzysztof
>

if you feel it's incorrect, I will drop this patch..

Thanks
-Anand

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [bootwrapper PATCH 1/2] aarch64: Disable trapping into EL3 while accessing MDSELR_EL1
From: Anshuman Khandual @ 2024-04-04  7:37 UTC (permalink / raw)
  To: linux-arm-kernel, mark.rutland; +Cc: Anshuman Khandual
In-Reply-To: <20240404073726.947215-1-anshuman.khandual@arm.com>

This disables trapping into EL3 while accessing MDSELR_EL1 either in EL1 or
EL2 via setting MDCR_EL3.EBWE. But first ensure that FEAT_Debugv8p9 feature
is implemented and enabled looking into ID_AA64DFR0_EL1.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 2 ++
 arch/aarch64/init.c            | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index d1f8fd9..124ef91 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -41,6 +41,7 @@
 #define MDCR_EL3_NSTB_NS_NOTRAP			(UL(3) << 24)
 #define MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT	(UL(3) << 32)
 #define MDCR_EL3_ENPMSN				BIT(36)
+#define MDCR_EL3_EBWE				BIT(43)
 
 #define SCR_EL3_RES1			BITS(5, 4)
 #define SCR_EL3_NS			BIT(0)
@@ -62,6 +63,7 @@
 #define ID_AA64DFR0_EL1_PMSVER		BITS(35, 32)
 #define ID_AA64DFR0_EL1_TRACEBUFFER	BITS(47, 44)
 #define ID_AA64DFR0_EL1_BRBE		BITS(55, 52)
+#define ID_AA64DFR0_EL1_DEBUGVER	BITS(3, 0)
 
 #define ID_AA64ISAR0_EL1_TME		BITS(27, 24)
 
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index c4e91e4..37cb45f 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -105,6 +105,9 @@ void cpu_init_el3(void)
 	if (mrs_field(ID_AA64DFR0_EL1, BRBE))
 		mdcr |= MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT;
 
+	if (mrs_field(ID_AA64DFR0_EL1, DEBUGVER) >= 11)
+		mdcr |= MDCR_EL3_EBWE;
+
 	msr(MDCR_EL3, mdcr);
 
 	if (mrs_field(ID_AA64PFR0_EL1, SVE)) {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [bootwrapper PATCH 2/2] aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers
From: Anshuman Khandual @ 2024-04-04  7:37 UTC (permalink / raw)
  To: linux-arm-kernel, mark.rutland; +Cc: Anshuman Khandual
In-Reply-To: <20240404073726.947215-1-anshuman.khandual@arm.com>

This disables trapping into EL3 while accessing Fine Grained Traps Enable 2
(i.e FEAT_FGT2) registers such as HDFGRTR2_EL2, HDFGWTR2_EL2, HFGITR2_EL2,
HFGRTR2_EL2 and HFGWTR2_EL2 via setting SCR_EL3.FGTEN2. But first ensure
that FEAT_FGT2 feature is implemented looking into ID_AA64MMFR0_EL1.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 1 +
 arch/aarch64/init.c            | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 124ef91..56f319a 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -57,6 +57,7 @@
 #define SCR_EL3_EnTP2			BIT(41)
 #define SCR_EL3_TCR2EN			BIT(43)
 #define SCR_EL3_PIEN			BIT(45)
+#define SCR_EL3_FGTEN2			BIT(59)
 
 #define HCR_EL2_RES1			BIT(1)
 
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 37cb45f..557266b 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -68,6 +68,9 @@ void cpu_init_el3(void)
 	if (mrs_field(ID_AA64MMFR0_EL1, FGT))
 		scr |= SCR_EL3_FGTEN;
 
+	if (mrs_field(ID_AA64MMFR0_EL1, FGT) >= 2)
+		scr |= SCR_EL3_FGTEN2;
+
 	if (mrs_field(ID_AA64MMFR0_EL1, ECV) >= 2)
 		scr |= SCR_EL3_ECVEN;
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [bootwrapper PATCH 0/2] aarch64: Prepare EL3 for MDSELR_EL1
From: Anshuman Khandual @ 2024-04-04  7:37 UTC (permalink / raw)
  To: linux-arm-kernel, mark.rutland; +Cc: Anshuman Khandual

MDSELR_EL1 register will be used to configure breakpoint and watchpoint
register banks when FEAT_Debugv8p9 is enabled. This series prepares EL3
for MDSELR_EL1 access traps into EL3, and into EL2 via new Fine Grained
Traps Enable 2 (FEAT_FGT2) registers.

Anshuman Khandual (2):
  aarch64: Disable trapping into EL3 while accessing MDSELR_EL1
  aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers

 arch/aarch64/include/asm/cpu.h | 3 +++
 arch/aarch64/init.c            | 6 ++++++
 2 files changed, 9 insertions(+)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] drm: xlnx: db: fix a memory leak in probe
From: Dan Carpenter @ 2024-04-04  7:32 UTC (permalink / raw)
  To: Rohit Visavalia
  Cc: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
	Vishal Sagar, Tomi Valkeinen, dri-devel, linux-arm-kernel,
	linux-kernel, kernel-janitors

Free "dp" before returning.

Fixes: be318d01a903 ("drm: xlnx: dp: Reset DisplayPort IP")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 5a40aa1d4283..8a15d18a65a6 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1716,7 +1716,7 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
 
 	ret = zynqmp_dp_reset(dp, true);
 	if (ret < 0)
-		return ret;
+		goto err_free;
 
 	ret = zynqmp_dp_reset(dp, false);
 	if (ret < 0)
-- 
2.43.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] coresight: tmc: Enable SG capability on ACPI based SoC-400 TMC ETR devices
From: Anshuman Khandual @ 2024-04-04  7:29 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: sudeep.holla, Anshuman Khandual, Mike Leach, James Clark,
	Alexander Shishkin, coresight, linux-kernel

This detects and enables the scatter gather capability (SG) on ACPI based
Soc-400 TMC ETR devices via a new property called 'arm-armhc97c-sg-enable'.
The updated ACPI spec can be found below, which contains this new property.

https://developer.arm.com/documentation/den0067/latest/

This preserves current handling for the property 'arm,scatter-gather' both
on ACPI and DT based platforms i.e the presence of the property is checked
instead of the value.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: coresight@lists.linaro.org 
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 .../hwtracing/coresight/coresight-tmc-core.c  | 28 ++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 72005b0c633e..2b277499b59a 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -4,6 +4,7 @@
  * Description: CoreSight Trace Memory Controller driver
  */
 
+#include <linux/acpi.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -360,7 +361,32 @@ static const struct attribute_group *coresight_etr_groups[] = {
 
 static inline bool tmc_etr_can_use_sg(struct device *dev)
 {
-	return fwnode_property_present(dev->fwnode, "arm,scatter-gather");
+	int ret;
+	u8 val_u8;
+
+	/*
+	 * Presence of the property 'arm,scatter-gather' is checked
+	 * on the platform for the feature support, rather than its
+	 * value.
+	 */
+	if (is_of_node(dev->fwnode)) {
+		return fwnode_property_present(dev->fwnode, "arm,scatter-gather");
+	} else if (is_acpi_device_node(dev->fwnode)) {
+		/*
+		 * TMC_DEVID_NOSCAT test in tmc_etr_setup_caps(), has already ensured
+		 * this property is only checked for Coresight SoC 400 TMC configured
+		 * as ETR.
+		 */
+		ret = fwnode_property_read_u8(dev->fwnode, "arm-armhc97c-sg-enable", &val_u8);
+		if (!ret)
+			return !!val_u8;
+
+		if (fwnode_property_present(dev->fwnode, "arm,scatter-gather")) {
+			pr_warn_once("Deprecated ACPI property - arm,scatter-gather\n");
+			return true;
+		}
+	}
+	return false;
 }
 
 static inline bool tmc_etr_has_non_secure_access(struct tmc_drvdata *drvdata)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] clk: rockchip: clk-rk3568.c: Add missing USB480M_PHY mux
From: Sascha Hauer @ 2024-04-04  7:27 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Heiko Stuebner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-clk, linux-arm-kernel, linux-rockchip, linux-kernel,
	devicetree, David Jander, Sascha Hauer

From: David Jander <david@protonic.nl>

The USB480M clock can source from a MUX that selects the clock to come
from either of the USB-phy internal 480MHz PLLs. These clocks are
provided by the USB phy driver.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/rockchip/clk-rk3568.c      | 4 ++++
 include/dt-bindings/clock/rk3568-cru.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/clk/rockchip/clk-rk3568.c b/drivers/clk/rockchip/clk-rk3568.c
index 8cb21d10beca2..2d44bcaef046b 100644
--- a/drivers/clk/rockchip/clk-rk3568.c
+++ b/drivers/clk/rockchip/clk-rk3568.c
@@ -215,6 +215,7 @@ static const struct rockchip_cpuclk_reg_data rk3568_cpuclk_data = {
 
 PNAME(mux_pll_p)			= { "xin24m" };
 PNAME(mux_usb480m_p)			= { "xin24m", "usb480m_phy", "clk_rtc_32k" };
+PNAME(mux_usb480m_phy_p)		= { "clk_usbphy0_480m", "clk_usbphy1_480m"};
 PNAME(mux_armclk_p)			= { "apll", "gpll" };
 PNAME(clk_i2s0_8ch_tx_p)		= { "clk_i2s0_8ch_tx_src", "clk_i2s0_8ch_tx_frac", "i2s0_mclkin", "xin_osc0_half" };
 PNAME(clk_i2s0_8ch_rx_p)		= { "clk_i2s0_8ch_rx_src", "clk_i2s0_8ch_rx_frac", "i2s0_mclkin", "xin_osc0_half" };
@@ -485,6 +486,9 @@ static struct rockchip_clk_branch rk3568_clk_branches[] __initdata = {
 	MUX(USB480M, "usb480m", mux_usb480m_p, CLK_SET_RATE_PARENT,
 			RK3568_MODE_CON0, 14, 2, MFLAGS),
 
+	MUX(USB480M_PHY, "usb480m_phy", mux_usb480m_phy_p, CLK_SET_RATE_PARENT,
+			RK3568_MISC_CON2, 15, 1, MFLAGS),
+
 	/* PD_CORE */
 	COMPOSITE(0, "sclk_core_src", apll_gpll_npll_p, CLK_IGNORE_UNUSED,
 			RK3568_CLKSEL_CON(2), 8, 2, MFLAGS, 0, 4, DFLAGS | CLK_DIVIDER_READ_ONLY,
diff --git a/include/dt-bindings/clock/rk3568-cru.h b/include/dt-bindings/clock/rk3568-cru.h
index d29890865150d..5263085c5b238 100644
--- a/include/dt-bindings/clock/rk3568-cru.h
+++ b/include/dt-bindings/clock/rk3568-cru.h
@@ -78,6 +78,7 @@
 #define CPLL_333M		9
 #define ARMCLK			10
 #define USB480M			11
+#define USB480M_PHY		12
 #define ACLK_CORE_NIU2BUS	18
 #define CLK_CORE_PVTM		19
 #define CLK_CORE_PVTM_CORE	20

---
base-commit: 39cd87c4eb2b893354f3b850f916353f2658ae6f
change-id: 20240404-clk-rockchip-rk3568-add-usb480m-phy-mux-882e40565a26

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2 5/6] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function
From: Krzysztof Kozlowski @ 2024-04-04  7:23 UTC (permalink / raw)
  To: Anand Moon, Thinh Nguyen, Greg Kroah-Hartman, Alim Akhtar
  Cc: Christophe JAILLET, Johan Hovold, linux-usb, linux-arm-kernel,
	linux-samsung-soc, linux-kernel
In-Reply-To: <20240404071350.4242-6-linux.amoon@gmail.com>

On 04/04/2024 09:13, Anand Moon wrote:
> Use devm_regulator_bulk_get_enable() instead of open coded
> 'devm_regulator_get(), regulator_enable(), regulator_disable().

I fail to see how did you replace open-coded suspend/resume paths.

> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> V2: no changes, did not find any regression in pm suspend/resume.

No, that's not equivalent code. No explanation in commit msg.

You already got comments on this and nothing improved. You just entirely
ignored received comments. That's not how it works.

I don't think you understand the code and Linux driver model. This patch
repeats several previous attempts with similar issues: no logic behind a
change.

NAK.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 3/6] usb: ohci-exynos: Use devm_clk_get_enabled() helpers
From: Krzysztof Kozlowski @ 2024-04-04  7:20 UTC (permalink / raw)
  To: Anand Moon, Alan Stern, Greg Kroah-Hartman, Alim Akhtar
  Cc: Christophe JAILLET, Johan Hovold, linux-usb, linux-arm-kernel,
	linux-samsung-soc, linux-kernel
In-Reply-To: <20240404071350.4242-4-linux.amoon@gmail.com>

On 04/04/2024 09:13, Anand Moon wrote:
> The devm_clk_get_enabled() helpers:
>     - call devm_clk_get()
>     - call clk_prepare_enable() and register what is needed in order to
>       call clk_disable_unprepare() when needed, as a managed resource.
> 
> This simplifies the code and avoids the calls to clk_disable_unprepare().
> 
> While at it, use dev_err_probe consistently, and use its return value
> to return the error code.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v2: new changes in this series.
> ---
>  drivers/usb/host/ohci-exynos.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 20e26a474591..85d04ae0ae40 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -135,20 +135,15 @@ static int exynos_ohci_probe(struct platform_device *pdev)
>  
>  	err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci);
>  	if (err)
> -		goto fail_clk;
> -
> -	exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
> +		goto fail_io;
>  
> +	exynos_ohci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost");
>  	if (IS_ERR(exynos_ohci->clk)) {
> -		dev_err(&pdev->dev, "Failed to get usbhost clock\n");
> -		err = PTR_ERR(exynos_ohci->clk);
> -		goto fail_clk;
> +		usb_put_hcd(hcd);
> +		return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ohci->clk),
> +				"Failed to get usbhost clock\n");

Why do you introduce entirely parallel exit paths? There is already
single error handling part with labels. Use that.



Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
From: Anand Moon @ 2024-04-04  7:13 UTC (permalink / raw)
  To: Thinh Nguyen, Greg Kroah-Hartman, Krzysztof Kozlowski,
	Alim Akhtar
  Cc: Anand Moon, Christophe JAILLET, Johan Hovold, linux-usb,
	linux-arm-kernel, linux-samsung-soc, linux-kernel
In-Reply-To: <20240404071350.4242-1-linux.amoon@gmail.com>

Use the new PM macros for the suspend and resume functions to be
automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
they will simply be discarded by the compiler.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
   disabled.
---
 drivers/usb/dwc3/dwc3-exynos.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 2d341f0e22a3..90259ad1d0d9 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -161,8 +161,7 @@ static const struct of_device_id exynos_dwc3_match[] = {
 };
 MODULE_DEVICE_TABLE(of, exynos_dwc3_match);
 
-#ifdef CONFIG_PM_SLEEP
-static int dwc3_exynos_suspend(struct device *dev)
+static int __maybe_unused dwc3_exynos_suspend(struct device *dev)
 {
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 	int i;
@@ -173,7 +172,7 @@ static int dwc3_exynos_suspend(struct device *dev)
 	return 0;
 }
 
-static int dwc3_exynos_resume(struct device *dev)
+static int __maybe_unused dwc3_exynos_resume(struct device *dev)
 {
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 	int i, ret;
@@ -194,18 +193,13 @@ static const struct dev_pm_ops dwc3_exynos_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(dwc3_exynos_suspend, dwc3_exynos_resume)
 };
 
-#define DEV_PM_OPS	(&dwc3_exynos_dev_pm_ops)
-#else
-#define DEV_PM_OPS	NULL
-#endif /* CONFIG_PM_SLEEP */
-
 static struct platform_driver dwc3_exynos_driver = {
 	.probe		= dwc3_exynos_probe,
 	.remove_new	= dwc3_exynos_remove,
 	.driver		= {
 		.name	= "exynos-dwc3",
 		.of_match_table = exynos_dwc3_match,
-		.pm	= DEV_PM_OPS,
+		.pm	= pm_sleep_ptr(&dwc3_exynos_dev_pm_ops),
 	},
 };
 
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 5/6] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function
From: Anand Moon @ 2024-04-04  7:13 UTC (permalink / raw)
  To: Thinh Nguyen, Greg Kroah-Hartman, Krzysztof Kozlowski,
	Alim Akhtar
  Cc: Anand Moon, Christophe JAILLET, Johan Hovold, linux-usb,
	linux-arm-kernel, linux-samsung-soc, linux-kernel
In-Reply-To: <20240404071350.4242-1-linux.amoon@gmail.com>

Use devm_regulator_bulk_get_enable() instead of open coded
'devm_regulator_get(), regulator_enable(), regulator_disable().

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
V2: no changes, did not find any regression in pm suspend/resume.
---
 drivers/usb/dwc3/dwc3-exynos.c | 50 ++++------------------------------
 1 file changed, 5 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 5d365ca51771..2d341f0e22a3 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -32,9 +32,6 @@ struct dwc3_exynos {
 	struct clk		*clks[DWC3_EXYNOS_MAX_CLOCKS];
 	int			num_clks;
 	int			suspend_clk_idx;
-
-	struct regulator	*vdd33;
-	struct regulator	*vdd10;
 };
 
 static int dwc3_exynos_probe(struct platform_device *pdev)
@@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
 	struct device_node	*node = dev->of_node;
 	const struct dwc3_exynos_driverdata *driver_data;
 	int			i, ret;
+	static const char * const regulators[] = { "vdd33", "vdd10" };
 
 	exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
 	if (!exynos)
@@ -78,27 +76,10 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
 	if (exynos->suspend_clk_idx >= 0)
 		clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
 
-	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
-	if (IS_ERR(exynos->vdd33)) {
-		ret = PTR_ERR(exynos->vdd33);
-		goto vdd33_err;
-	}
-	ret = regulator_enable(exynos->vdd33);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD33 supply\n");
-		goto vdd33_err;
-	}
-
-	exynos->vdd10 = devm_regulator_get(dev, "vdd10");
-	if (IS_ERR(exynos->vdd10)) {
-		ret = PTR_ERR(exynos->vdd10);
-		goto vdd10_err;
-	}
-	ret = regulator_enable(exynos->vdd10);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD10 supply\n");
-		goto vdd10_err;
-	}
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
+					     regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
 
 	if (node) {
 		ret = of_platform_populate(node, NULL, NULL, dev);
@@ -115,10 +96,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
 	return 0;
 
 populate_err:
-	regulator_disable(exynos->vdd10);
-vdd10_err:
-	regulator_disable(exynos->vdd33);
-vdd33_err:
 	for (i = exynos->num_clks - 1; i >= 0; i--)
 		clk_disable_unprepare(exynos->clks[i]);
 
@@ -140,9 +117,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
 
 	if (exynos->suspend_clk_idx >= 0)
 		clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
-
-	regulator_disable(exynos->vdd33);
-	regulator_disable(exynos->vdd10);
 }
 
 static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
@@ -196,9 +170,6 @@ static int dwc3_exynos_suspend(struct device *dev)
 	for (i = exynos->num_clks - 1; i >= 0; i--)
 		clk_disable_unprepare(exynos->clks[i]);
 
-	regulator_disable(exynos->vdd33);
-	regulator_disable(exynos->vdd10);
-
 	return 0;
 }
 
@@ -207,17 +178,6 @@ static int dwc3_exynos_resume(struct device *dev)
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 	int i, ret;
 
-	ret = regulator_enable(exynos->vdd33);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD33 supply\n");
-		return ret;
-	}
-	ret = regulator_enable(exynos->vdd10);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD10 supply\n");
-		return ret;
-	}
-
 	for (i = 0; i < exynos->num_clks; i++) {
 		ret = clk_prepare_enable(exynos->clks[i]);
 		if (ret) {
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 4/6] usb: ohci-exynos: Switch from CONFIG_PM guards to pm_ptr()
From: Anand Moon @ 2024-04-04  7:13 UTC (permalink / raw)
  To: Alan Stern, Greg Kroah-Hartman, Krzysztof Kozlowski, Alim Akhtar
  Cc: Anand Moon, Christophe JAILLET, Johan Hovold, linux-usb,
	linux-arm-kernel, linux-samsung-soc, linux-kernel
In-Reply-To: <20240404071350.4242-1-linux.amoon@gmail.com>

Use the new PM macros for the suspend and resume functions to be
automatically dropped by the compiler when CONFIG_PM are disabled,
without having to use #ifdef guards. If CONFIG_PM unused,
they will simply be discarded by the compiler.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2: new changes in this series.
---
 drivers/usb/host/ohci-exynos.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 85d04ae0ae40..3e647e0b341d 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -212,8 +212,7 @@ static void exynos_ohci_shutdown(struct platform_device *pdev)
 		hcd->driver->shutdown(hcd);
 }
 
-#ifdef CONFIG_PM
-static int exynos_ohci_suspend(struct device *dev)
+static int __maybe_unused exynos_ohci_suspend(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
 	struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
@@ -230,7 +229,7 @@ static int exynos_ohci_suspend(struct device *dev)
 	return 0;
 }
 
-static int exynos_ohci_resume(struct device *dev)
+static int __maybe_unused exynos_ohci_resume(struct device *dev)
 {
 	struct usb_hcd *hcd			= dev_get_drvdata(dev);
 	struct exynos_ohci_hcd *exynos_ohci	= to_exynos_ohci(hcd);
@@ -249,10 +248,6 @@ static int exynos_ohci_resume(struct device *dev)
 
 	return 0;
 }
-#else
-#define exynos_ohci_suspend	NULL
-#define exynos_ohci_resume	NULL
-#endif
 
 static const struct ohci_driver_overrides exynos_overrides __initconst = {
 	.extra_priv_size =	sizeof(struct exynos_ohci_hcd),
@@ -277,7 +272,7 @@ static struct platform_driver exynos_ohci_driver = {
 	.shutdown	= exynos_ohci_shutdown,
 	.driver = {
 		.name	= "exynos-ohci",
-		.pm	= &exynos_ohci_pm_ops,
+		.pm	= pm_ptr(&exynos_ohci_pm_ops),
 		.of_match_table	= of_match_ptr(exynos_ohci_match),
 	}
 };
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox