On Tue, Jul 26, 2016 at 10:52:06AM +0530, kamlakant.patel@xxxxxxxxxxxx wrote:
> - if (IS_ERR(clk)) {
> - dev_err(&pdev->dev, "could not get spi clock\n");
> - return -ENODEV;
> + if (!IS_ERR(clk)) {
> + xspi->spi_clk = clk_get_rate(clk);
> + } else {
> + u32 freq;
> +
> + err = device_property_read_u32(&pdev->dev,
> + "clock-frequency", &freq);
Ugh, this is horrible, especially with...
> + if (err) {
> + /*
> + * Use default SPI frequency for Vulcan unless
> + * changed by firmaware.
> + */
> + dev_err(&pdev->dev, "Can't get clock-frequency,
> + using default\n");
> + freq = VULCAN_SPI_DEFAULT_FREQ;
...this just completely ignoring error handling (it at least needs
fixing for probe deferral). This actively breaks things for existing
users of the driver. Splitting the log message up like that is also
very bad, the log message will end up with a newline and a huge number
of spaces in the middle of it.
This should be specific to the relevant systems, not unconditionally
changed for all users, and should be implemented by registering a fixed
clock on the relevant platforms like spi-pxa2xx does on Intel systems so
that the driver works in the same manner outside of probe on everything.
Attachment:
signature.asc
Description: PGP signature
