davidker / unisys

Master repository for new changes to drivers/staging/unisys and drivers/visorbus
Other
2 stars 1 forks source link

Cleanup and document CONTROLVM response error messages #133

Open wadgaonkarsam opened 7 years ago

wadgaonkarsam commented 7 years ago
static void
chipset_init(struct controlvm_message *inmsg)
{
    static int chipset_inited;
    enum ultra_chipset_feature features = 0;
    int rc = CONTROLVM_RESP_SUCCESS;

What's with the use of this odd return value in lots of places?

It should just be 0 or a -error number. Don't mess with odd non-standard values. You do that a lot in this file.

    if (chipset_inited) {
        rc = -CONTROLVM_RESP_ERROR_ALREADY_DONE;

See, odd errors :(

         */
        pmsg_hdr = msg_hdr;
        goto out_respond;
    }

    if (dev_info->pending_msg_hdr) {
        /* only non-NULL if dev is still waiting on a response */
        response = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
        pmsg_hdr = dev_info->pending_msg_hdr;
        goto out_respond;
    }

    if (need_response) {
        pmsg_hdr = kzalloc(sizeof(*pmsg_hdr), GFP_KERNEL);
        if (!pmsg_hdr) {
            response = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;

-ENOMEM.

See, saved you a bunch of characters. You're welcome.

    }

    INIT_LIST_HEAD(&bus_info->list_all);
    bus_info->chipset_bus_no = bus_no;
    bus_info->chipset_dev_no = BUS_ROOT_DEVICE;

    POSTCODE_LINUX_3(BUS_CREATE_ENTRY_PC, bus_no, POSTCODE_SEVERITY_INFO);

    visorchannel = visorchannel_create(cmd->create_bus.channel_addr,
                       cmd->create_bus.channel_bytes,
                       GFP_KERNEL,
                       cmd->create_bus.bus_data_type_uuid);

    if (!visorchannel) {
        POSTCODE_LINUX_3(BUS_CREATE_FAILURE_PC, bus_no,
                 POSTCODE_SEVERITY_ERR);
        rc = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
        kfree(bus_info);
        bus_info = NULL;
        goto out_bus_epilog;
    }
    bus_info->visorchannel = visorchannel;
    if (uuid_le_cmp(cmd->create_bus.bus_inst_uuid, spar_siovm_uuid) == 0) {
        dump_vhba_bus = bus_no;
        save_crash_message(inmsg, CRASH_BUS);
    }

    POSTCODE_LINUX_3(BUS_CREATE_EXIT_PC, bus_no, POSTCODE_SEVERITY_INFO);

out_bus_epilog:
    bus_epilog(bus_info, CONTROLVM_BUS_CREATE, &inmsg->hdr,
           rc, inmsg->hdr.flags.response_expected == 1);
}

static void
bus_destroy(struct controlvm_message *inmsg)
{
    struct controlvm_message_packet *cmd = &inmsg->cmd;
    u32 bus_no = cmd->destroy_bus.bus_no;
    struct visor_device *bus_info;
    int rc = CONTROLVM_RESP_SUCCESS;

    bus_info = visorbus_get_device_by_id(bus_no, BUS_ROOT_DEVICE, NULL);
    if (!bus_info)
        rc = -CONTROLVM_RESP_ERROR_BUS_INVALID;

And again with the funky errors, just delete them all, this is the kernel, use the ones we have, don't make driver/subsystem-specific ones.

    if (!payload)
        return -CONTROLVM_RESP_ERROR_IOREMAP_FAILED;

Again with the funky error codes, I'll just stop saying them now, please delete all of them and use the standard kernel errors in all of the visorbus codebase.


    info->offset = offset;
    info->bytes = bytes;
    info->ptr = payload;

    return CONTROLVM_RESP_SUCCESS;

0 {sniff}

    return CONTROLVM_RESP_SUCCESS;

And 0, really. You should have noticed that the whole kernel does this, you aren't special. Well, you are, we all are special and unique, just like everyone else...

    return CONTROLVM_RESP_SUCCESS;
  1. Come on...