Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1207984 > unrolled thread

Re: [PATCH V3 4/6] slim: qcom: Add Qualcomm Slimbus controller driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V3 4/6] slim: qcom: Add Qualcomm Slimbus controller driver Mark Brown <broonie@kernel.org> - 2015-08-15 04:20 +0200
    Re: [PATCH V3 4/6] slim: qcom: Add Qualcomm Slimbus controller driver Sagar Dharia <sdharia@codeaurora.org> - 2015-08-18 01:50 +0200

#1207984 — Re: [PATCH V3 4/6] slim: qcom: Add Qualcomm Slimbus controller driver

[Multipart message — attachments visible in raw view] — view raw

On Mon, Aug 03, 2015 at 12:59:48AM -0600, Sagar Dharia wrote:

> +/**
> + * Qualcomm slimbus controller may experience interference on the lines
> + * causing some early messages (e.g. logical-address assignment) to be
> + * NACKed. Retry after sometime (typically 1 superframe)
> + */
> +static void msm_slim_wait_retry(struct msm_slim_ctrl *dev)
> +{
> +	int msec_per_frm = 0;
> +	int sfr_per_sec;
> +
> +	/* Wait for 1 superframe, or default time and then retry */
> +	sfr_per_sec = dev->framer.superfreq /
> +			(1 << (SLIM_MAX_CLK_GEAR - dev->ctrl.clkgear));
> +	if (sfr_per_sec)
> +		msec_per_frm = MSEC_PER_SEC / sfr_per_sec;
> +	if (msec_per_frm < DEF_RETRY_MS)
> +		msec_per_frm = DEF_RETRY_MS;
> +	msleep(msec_per_frm);
> +}

This looks like logic which might be usefully pulled out to the
framework - I bet this isn't an issue unique to your controller, the
users (well, one user) look like cases where the bus might be
destabilised by devices powering up and trying to enumerate under less
than ideal electrical conditions which seems like something that's
likely to affect other hardware.

> +	rclk = devm_clk_get(&pdev->dev, "core_clk");
> +	if (IS_ERR(rclk)) {
> +		/* unlikely that this is probe-defer */
> +		dev_err(&pdev->dev, "rclk get failed:%ld\n", PTR_ERR(rclk));
> +		devm_clk_put(&pdev->dev, hclk);

No need to call devm_clk_put() explicitly in your probe function.

[toc] | [next] | [standalone]

#1208957

On 8/14/2015 12:31 PM, Mark Brown wrote:
> On Mon, Aug 03, 2015 at 12:59:48AM -0600, Sagar Dharia wrote:
>
>> +/**
>> + * Qualcomm slimbus controller may experience interference on the lines
>> + * causing some early messages (e.g. logical-address assignment) to be
>> + * NACKed. Retry after sometime (typically 1 superframe)
>> + */
>> +static void msm_slim_wait_retry(struct msm_slim_ctrl *dev)
>> +{
>> +	int msec_per_frm = 0;
>> +	int sfr_per_sec;
>> +
>> +	/* Wait for 1 superframe, or default time and then retry */
>> +	sfr_per_sec = dev->framer.superfreq /
>> +			(1 << (SLIM_MAX_CLK_GEAR - dev->ctrl.clkgear));
>> +	if (sfr_per_sec)
>> +		msec_per_frm = MSEC_PER_SEC / sfr_per_sec;
>> +	if (msec_per_frm < DEF_RETRY_MS)
>> +		msec_per_frm = DEF_RETRY_MS;
>> +	msleep(msec_per_frm);
>> +}
> This looks like logic which might be usefully pulled out to the
> framework - I bet this isn't an issue unique to your controller, the
> users (well, one user) look like cases where the bus might be
> destabilised by devices powering up and trying to enumerate under less
> than ideal electrical conditions which seems like something that's
> likely to affect other hardware.

I agree, I will move this to the framework.

Thanks
Sagar

>
>> +	rclk = devm_clk_get(&pdev->dev, "core_clk");
>> +	if (IS_ERR(rclk)) {
>> +		/* unlikely that this is probe-defer */
>> +		dev_err(&pdev->dev, "rclk get failed:%ld\n", PTR_ERR(rclk));
>> +		devm_clk_put(&pdev->dev, hclk);
> No need to call devm_clk_put() explicitly in your probe function.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web