Hi Oleksij,
Some small nitpicks inside.
On Sat, Jun 22, 2024 at 07:48:15PM +0200, Oleksij Rempel wrote:
> +++ b/arch/arm/boards/microchip-sama5d3-eds/board.c
> @@ -0,0 +1,414 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-FileCopyrightText: 2024 Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> +
> +#include <common.h>
> +#include <deep-probe.h>
> +#include <envfs.h>
> +#include <environment.h>
> +#include <gpio.h>
> +#include <init.h>
> +#include <linux/phy.h>
> +#include <spi/spi.h>
> +
> +#define BASE_BOARD_NAME_SIZE 100
> +static char base_board_name[BASE_BOARD_NAME_SIZE] = "MCP_S5D3_EDS";
> +
> +extern char __dtbo_sama5d3_eds_rgmii_phy_start[];
> +extern char __dtbo_sama5d3_eds_lan9370_start[];
> +
> +#define MCP_S5D3_EDS_MDIO_EXT_BOARD_FLAG_IGNGORE BIT(0)
s/IGNGORE/IGNORE/
> +static int mcp_s5d3_eds_spi_find_ksz_switch(struct device *dev, int bus_idx,
> + int chip_select, u8 *response,
> + size_t response_size)
> +{
> + struct device_node *spi_node;
> + struct spi_device spi = {
> + .max_speed_hz = 1 * 1000 * 1000, /* 1 MHz */
> + .bits_per_word = 8,
> + .chip_select = chip_select,
> + };
> + /* read command for ksz switch */
> + u8 tx_buf[4] = {0x60, 0x00, 0x00, 0x00};
> + u8 rx_buf[4] = {0};
> + int ret, i;
> + bool all_zeros = true, all_ones = true;
> + char spi_alias[] = "spiX";
> +
> + snprintf(spi_alias, sizeof(spi_alias), "spi%d", bus_idx);
> +
> + spi_node = of_find_node_by_alias(NULL, spi_alias);
> + if (!spi_node)
> + return -ENODEV;
> +
> + of_device_ensure_probed(spi_node);
> +
> + spi.controller = spi_get_controller(bus_idx);
> + spi.master = spi.controller;
> +
> + if (!spi.controller) {
> + dev_err(dev, "SPI bus %d not found\n", bus_idx);
> + return -ENODEV;
> + }
> +
> + ret = spi.controller->setup(&spi);
> + if (ret) {
> + dev_err(dev, "Cannot setup SPI controller %d (%d)\n", bus_idx,
> + ret);
You can use ": %pe\n", ERR_PTR(ret) here.
> + return ret;
> + }
> +
> + ret = spi_write_then_read(&spi, tx_buf, sizeof(tx_buf), rx_buf,
> + sizeof(rx_buf));
> + if (ret) {
> + dev_err(dev, "Failed to communicate with SPI device on bus %d chip select %d\n",
> + bus_idx, chip_select);
> + return ret;
> + }
> +
> + for (i = 0; i < sizeof(rx_buf); i++) {
> + if (rx_buf[i] != 0x00) {
> + all_zeros = false;
> + }
> + if (rx_buf[i] != 0xFF) {
> + all_ones = false;
> + }
> + }
> +
> + if (all_zeros || all_ones)
> + return 0;
> +
> + if (response_size < sizeof(rx_buf)) {
> + dev_err(dev, "Response storage is too small\n");
No need to print this message, it's more a sanity check if your code is
correct. Also, you can move this up and catch this before you are
sending SPI messages.
> + scan->spi_bus_idx,
> + scan->spi_chip_select,
> + response);
> + if (!found)
> + dev_warn(dev, "No match for SPI device on bus %d chip select %d, response: %02x %02x %02x %02x\n",
> + scan->spi_bus_idx, scan->spi_chip_select,
> + response[0], response[1], response[2],
> + response[3]);
printf can print (small) hexdumps now using "%*ph" whic comes in handy
here.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
