> +static int mipi_csis_clk_get(struct csi_state *state)
> +{
> + struct device *dev = &state->pdev->dev;
> + int ret = true;
Better to leave ret unitialized.
> +
> + state->mipi_clk = devm_clk_get(dev, "mipi");
> + if (IS_ERR(state->mipi_clk)) {
> + dev_err(dev, "Could not get mipi csi clock\n");
> + return -ENODEV;
> + }
> +
> + state->phy_clk = devm_clk_get(dev, "phy");
> + if (IS_ERR(state->phy_clk)) {
> + dev_err(dev, "Could not get mipi phy clock\n");
> + return -ENODEV;
> + }
> +
> + /* Set clock rate */
> + if (state->clk_frequency)
> + ret = clk_set_rate(state->mipi_clk, state->clk_frequency);
> + else
> + dev_warn(dev, "No clock frequency specified!\n");
> + if (ret < 0) {
> + dev_err(dev, "set rate=%d failed: %d\n", state->clk_frequency,
> + ret);
> + return -EINVAL;
Preserve the error code.
> + }
> +
> + return ret;
This could be "return 0;" (let's not return true). It might be nicer
like:
if (!state->clk_frequency) {
dev_warn(dev, "No clock frequency specified!\n");
return 0;
}
ret = clk_set_rate(state->mipi_clk, state->clk_frequency);
if (ret < 0)
dev_err(dev, "set rate=%d failed: %d\n", state->clk_frequency,
ret);
return ret;
> +}
> +
[ snip ]
> +static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id)
> +{
> + struct csi_state *state = dev_id;
> + unsigned long flags;
> + u32 status;
> + int i;
> +
> + status = mipi_csis_read(state, MIPI_CSIS_INTSRC);
> +
> + spin_lock_irqsave(&state->slock, flags);
> +
> + /* Update the event/error counters */
> + if ((status & MIPI_CSIS_INTSRC_ERRORS) || 1) {
^^^
Was this supposed to make it into the published code?
> + for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) {
> + if (!(status & state->events[i].mask))
> + continue;
> + state->events[i].counter++;
> + }
> + }
> + spin_unlock_irqrestore(&state->slock, flags);
> +
> + mipi_csis_write(state, MIPI_CSIS_INTSRC, status);
> +
> + return IRQ_HANDLED;
> +}
> +
regards,
dan carpenter
