Unverified Commit eaa59db7 authored by Mark Brown's avatar Mark Brown
Browse files

Add dev_warn_probe() and improve error handling in 

Merge series from Dragan Simic <dsimic@manjaro.org>:

This is a small series that introduces dev_warn_probe() function, which
produces warnings on failed resource acquisitions, and improves error
handling in the probe paths of Rockchip SPI drivers, by using functions
dev_err_probe() and dev_warn_probe() properly in multiple places.

This series also performs a bunch of small, rather trivial code cleanups,
to make the code neater and a bit easier to read.
parents b1258105 e2fc0587

drivers/base/core.c

+101 −28
Original line number Diff line number Diff line
@@ -4980,6 +4980,49 @@ define_dev_printk_level(_dev_info, KERN_INFO); 


#endif



static void __dev_probe_failed(const struct device *dev, int err, bool fatal,

			       const char *fmt, va_list vargsp)

{

	struct va_format vaf;

	va_list vargs;



	/*

	 * On x86_64 and possibly on other architectures, va_list is actually a

	 * size-1 array containing a structure.  As a result, function parameter

	 * vargsp decays from T[1] to T*, and &vargsp has type T** rather than

	 * T(*)[1], which is expected by its assignment to vaf.va below.

	 *

	 * One standard way to solve this mess is by creating a copy in a local

	 * variable of type va_list and then using a pointer to that local copy

	 * instead, which is the approach employed here.

	 */

	va_copy(vargs, vargsp);



	vaf.fmt = fmt;

	vaf.va = &vargs;



	switch (err) {

	case -EPROBE_DEFER:

		device_set_deferred_probe_reason(dev, &vaf);

		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);

		break;



	case -ENOMEM:

		/* Don't print anything on -ENOMEM, there's already enough output */

		break;



	default:

		/* Log fatal final failures as errors, otherwise produce warnings */

		if (fatal)

			dev_err(dev, "error %pe: %pV", ERR_PTR(err), &vaf);

		else

			dev_warn(dev, "error %pe: %pV", ERR_PTR(err), &vaf);

		break;

	}



	va_end(vargs);

}



/**

 * dev_err_probe - probe error check and log helper

 * @dev: the pointer to the struct device
@@ -4992,7 +5035,7 @@ define_dev_printk_level(_dev_info, KERN_INFO); 
 * -EPROBE_DEFER and propagate error upwards.

 * In case of -EPROBE_DEFER it sets also defer probe reason, which can be

 * checked later by reading devices_deferred debugfs attribute.

 * It replaces code sequence::

 * It replaces the following code sequence::

 *

 * 	if (err != -EPROBE_DEFER)

 * 		dev_err(dev, ...);
@@ -5004,47 +5047,77 @@ define_dev_printk_level(_dev_info, KERN_INFO); 
 *

 * 	return dev_err_probe(dev, err, ...);

 *

 * Using this helper in your probe function is totally fine even if @err is

 * known to never be -EPROBE_DEFER.

 * Using this helper in your probe function is totally fine even if @err

 * is known to never be -EPROBE_DEFER.

 * The benefit compared to a normal dev_err() is the standardized format

 * of the error code, it being emitted symbolically (i.e. you get "EAGAIN"

 * instead of "-35") and the fact that the error code is returned which allows

 * more compact error paths.

 * of the error code, which is emitted symbolically (i.e. you get "EAGAIN"

 * instead of "-35"), and having the error code returned allows more

 * compact error paths.

 *

 * Returns @err.

 */

int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)

{

	struct va_format vaf;

	va_list args;

	va_list vargs;



	va_start(args, fmt);

	vaf.fmt = fmt;

	vaf.va = &args;

	va_start(vargs, fmt);



	switch (err) {

	case -EPROBE_DEFER:

		device_set_deferred_probe_reason(dev, &vaf);

		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);

		break;

	/* Use dev_err() for logging when err doesn't equal -EPROBE_DEFER */

	__dev_probe_failed(dev, err, true, fmt, vargs);



	case -ENOMEM:

		/*

		 * We don't print anything on -ENOMEM, there is already enough

		 * output.

		 */

		break;

	va_end(vargs);



	default:

		dev_err(dev, "error %pe: %pV", ERR_PTR(err), &vaf);

		break;

	return err;

}

EXPORT_SYMBOL_GPL(dev_err_probe);



	va_end(args);

/**

 * dev_warn_probe - probe error check and log helper

 * @dev: the pointer to the struct device

 * @err: error value to test

 * @fmt: printf-style format string

 * @...: arguments as specified in the format string

 *

 * This helper implements common pattern present in probe functions for error

 * checking: print debug or warning message depending if the error value is

 * -EPROBE_DEFER and propagate error upwards.

 * In case of -EPROBE_DEFER it sets also defer probe reason, which can be

 * checked later by reading devices_deferred debugfs attribute.

 * It replaces the following code sequence::

 *

 * 	if (err != -EPROBE_DEFER)

 * 		dev_warn(dev, ...);

 * 	else

 * 		dev_dbg(dev, ...);

 * 	return err;

 *

 * with::

 *

 * 	return dev_warn_probe(dev, err, ...);

 *

 * Using this helper in your probe function is totally fine even if @err

 * is known to never be -EPROBE_DEFER.

 * The benefit compared to a normal dev_warn() is the standardized format

 * of the error code, which is emitted symbolically (i.e. you get "EAGAIN"

 * instead of "-35"), and having the error code returned allows more

 * compact error paths.

 *

 * Returns @err.

 */

int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)

{

	va_list vargs;



	va_start(vargs, fmt);



	/* Use dev_warn() for logging when err doesn't equal -EPROBE_DEFER */

	__dev_probe_failed(dev, err, false, fmt, vargs);



	va_end(vargs);



	return err;

}

EXPORT_SYMBOL_GPL(dev_err_probe);

EXPORT_SYMBOL_GPL(dev_warn_probe);



static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)

{

drivers/spi/spi-rockchip.c

+13 −15
Original line number Diff line number Diff line
@@ -773,15 +773,15 @@ static int rockchip_spi_probe(struct platform_device *pdev) 


	rs->apb_pclk = devm_clk_get_enabled(&pdev->dev, "apb_pclk");

	if (IS_ERR(rs->apb_pclk)) {

		dev_err(&pdev->dev, "Failed to get apb_pclk\n");

		ret = PTR_ERR(rs->apb_pclk);

		ret = dev_err_probe(&pdev->dev, PTR_ERR(rs->apb_pclk),

				    "Failed to get apb_pclk\n");

		goto err_put_ctlr;

	}



	rs->spiclk = devm_clk_get_enabled(&pdev->dev, "spiclk");

	if (IS_ERR(rs->spiclk)) {

		dev_err(&pdev->dev, "Failed to get spi_pclk\n");

		ret = PTR_ERR(rs->spiclk);

		ret = dev_err_probe(&pdev->dev, PTR_ERR(rs->spiclk),

				    "Failed to get spi_pclk\n");

		goto err_put_ctlr;

	}
@@ -817,8 +817,7 @@ static int rockchip_spi_probe(struct platform_device *pdev) 


	rs->fifo_len = get_fifo_len(rs);

	if (!rs->fifo_len) {

		dev_err(&pdev->dev, "Failed to get fifo length\n");

		ret = -EINVAL;

		ret = dev_err_probe(&pdev->dev, -EINVAL, "Failed to get fifo length\n");

		goto err_put_ctlr;

	}
@@ -858,22 +857,21 @@ static int rockchip_spi_probe(struct platform_device *pdev) 


	ctlr->dma_tx = dma_request_chan(rs->dev, "tx");

	if (IS_ERR(ctlr->dma_tx)) {

		/* Check tx to see if we need defer probing driver */

		if (PTR_ERR(ctlr->dma_tx) == -EPROBE_DEFER) {

			ret = -EPROBE_DEFER;

		/* Check tx to see if we need to defer driver probing */

		ret = dev_warn_probe(rs->dev, PTR_ERR(ctlr->dma_tx),

				     "Failed to request optional TX DMA channel\n");

		if (ret == -EPROBE_DEFER)

			goto err_disable_pm_runtime;

		}

		dev_warn(rs->dev, "Failed to request TX DMA channel\n");

		ctlr->dma_tx = NULL;

	}



	ctlr->dma_rx = dma_request_chan(rs->dev, "rx");

	if (IS_ERR(ctlr->dma_rx)) {

		if (PTR_ERR(ctlr->dma_rx) == -EPROBE_DEFER) {

			ret = -EPROBE_DEFER;

		/* Check rx to see if we need to defer driver probing */

		ret = dev_warn_probe(rs->dev, PTR_ERR(ctlr->dma_rx),

				     "Failed to request optional RX DMA channel\n");

		if (ret == -EPROBE_DEFER)

			goto err_free_dma_tx;

		}

		dev_warn(rs->dev, "Failed to request RX DMA channel\n");

		ctlr->dma_rx = NULL;

	}

include/linux/dev_printk.h

+1 −0
Original line number Diff line number Diff line
@@ -276,6 +276,7 @@ do { \ 
			dev_driver_string(dev), dev_name(dev), ## arg)



__printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);

__printf(3, 4) int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...);



/* Simple helper for dev_err_probe() when ERR_PTR() is to be returned. */

#define dev_err_ptr_probe(dev, ___err, fmt, ...) \