On Mon, Sep 14, 2020 at 11:04:42AM +0800, Henry Chen wrote: > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2019 MediaTek Inc. > + */ Please make the entire comment a C++ one so things look more intentional, and are you sure about that copyright date? > +static int dvfsrc_set_voltage_sel(struct regulator_dev *rdev, > + unsigned int selector) > +{ > + struct device *dvfsrc_dev = to_dvfsrc_dev(rdev); > + int id = rdev_get_id(rdev); > + > + switch (id) { > + case DVFSRC_ID_VCORE: > + mtk_dvfsrc_send_request(dvfsrc_dev, > + MTK_DVFSRC_CMD_VCORE_REQUEST, > + selector); > + break; > + case DVFSRC_ID_VSCP: > + mtk_dvfsrc_send_request(dvfsrc_dev, > + MTK_DVFSRC_CMD_VSCP_REQUEST, > + selector); > + break; Just have two separate operations rather than the switch statement, and where you do have switch statements please follow the kernel coding style.