On Mon, Mar 08, 2021 at 12:05:22PM -0500, Al Cooper wrote:
> +static void brcmuart_init_debugfs(struct brcmuart_priv *priv,
> + const char *device)
> +{
> + priv->debugfs_dir = debugfs_create_dir(device, brcmuart_debugfs_root);
> + priv->debugfs_file = debugfs_create_file("stats", 0444,
> + priv->debugfs_dir,
> + priv,
> + &debugfs_stats_fops);
No need to save this file *, you never do anything with it.
> +}
> +
> +
> +static int brcmuart_probe(struct platform_device *pdev)
> +{
> + struct resource *regs;
> + struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *of_id = NULL;
> + struct uart_8250_port *new_port;
> + struct device *dev = &pdev->dev;
> + struct brcmuart_priv *priv;
> + struct clk *baud_mux_clk;
> + struct uart_8250_port up;
> + struct resource *irq;
> + void __iomem *membase = 0;
> + resource_size_t mapbase = 0;
> + u32 clk_rate = 0;
> + int ret;
> + int x;
> + int dma_irq;
> + static const char * const reg_names[REGS_MAX] = {
> + "uart", "dma_rx", "dma_tx", "dma_intr2", "dma_arb"
> + };
> +
> + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!irq) {
> + dev_err(dev, "missing irq\n");
> + return -EINVAL;
> + }
> + priv = devm_kzalloc(dev, sizeof(struct brcmuart_priv),
> + GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + of_id = of_match_node(brcmuart_dt_ids, np);
> + if (!of_id || !of_id->data)
> + priv->rate_table = brcmstb_rate_table;
> + else
> + priv->rate_table = of_id->data;
> +
> + for (x = 0; x < REGS_MAX; x++) {
> + regs = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + reg_names[x]);
> + if (!regs)
> + break;
> + priv->regs[x] = devm_ioremap(dev, regs->start,
> + resource_size(regs));
> + if (IS_ERR(priv->regs[x]))
> + return PTR_ERR(priv->regs[x]);
> + if (x == REGS_8250) {
> + mapbase = regs->start;
> + membase = priv->regs[x];
> + }
> + }
> +
> + /* We should have just the uart base registers or all the registers */
> + if (x != 1 && x != REGS_MAX) {
> + dev_warn(dev, "%s registers not specified\n", reg_names[x]);
> + return -EINVAL;
> + }
> +
> + /* if the DMA registers were specified, try to enable DMA */
> + if (x > REGS_DMA_RX) {
> + if (brcmuart_arbitration(priv, 1) == 0) {
> + u32 txrev = 0;
> + u32 rxrev = 0;
> +
> + txrev = udma_readl(priv, REGS_DMA_RX, UDMA_RX_REVISION);
> + rxrev = udma_readl(priv, REGS_DMA_TX, UDMA_TX_REVISION);
> + if ((txrev >= UDMA_TX_REVISION_REQUIRED) &&
> + (rxrev >= UDMA_RX_REVISION_REQUIRED)) {
> +
> + /* Enable the use of the DMA hardware */
> + priv->dma_enabled = true;
> + } else {
> + brcmuart_arbitration(priv, 0);
> + dev_err(dev,
> + "Unsupported DMA Hardware Revision\n");
> + }
> + } else {
> + dev_err(dev,
> + "Timeout arbitrating for UART DMA hardware\n");
> + }
> + }
> +
> + of_property_read_u32(np, "clock-frequency", &clk_rate);
> +
> + /* See if a Baud clock has been specified */
> + baud_mux_clk = of_clk_get_by_name(np, "sw_baud");
> + if (IS_ERR(baud_mux_clk)) {
> + if (PTR_ERR(baud_mux_clk) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_info(dev, "BAUD MUX clock not specified\n");
> + } else {
> + dev_info(dev, "BAUD MUX clock found\n");
These are not "info" messages, make them dev_dbg() please.
> + ret = clk_prepare_enable(baud_mux_clk);
> + if (ret)
> + return ret;
> + priv->baud_mux_clk = baud_mux_clk;
> + init_real_clk_rates(dev, priv);
> + clk_rate = priv->default_mux_rate;
> + }
> +
> + if (clk_rate == 0) {
> + dev_err(dev, "clock-frequency or clk not defined\n");
> + return -EINVAL;
> + }
> +
> + dev_info(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
When a driver is working properly, no need to say anything in the kernel
log.
This should be dev_info().
Much better overall, thanks for making the changes.
thanks,
greg k-h
